Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Add Wrapped ETH Support #81

Merged
merged 8 commits into from
Jun 26, 2017
Merged

Add Wrapped ETH Support #81

merged 8 commits into from
Jun 26, 2017

Conversation

fabioberger
Copy link
Contributor

This PR:

  • Adds support for calling deposit and withdraw on an Ether Wrapper ERC20 token contract.
  • Adds tests for testing both methods and that the correct error messages are thrown if the caller has an insufficient balance or ETH/WETH.
  • Adds a missing return comment for getBalanceAsync.


const wethContractAddress = await this.getContractAddressAsync();
const WETHBalanceInBaseUnits = await this._tokenWrapper.getBalanceAsync(wethContractAddress, withdrawer);
assert.assert(WETHBalanceInBaseUnits.gte(amountInWei), ZeroExError.INSUFFICIENT_WETH_BALANCE_FOR_WITHDRAWL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WITHDRAWAL

const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle();

describe.only('EtherTokenWrapper', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove only

public toWei(ethAmount: BigNumber.BigNumber): BigNumber.BigNumber {
const balanceWei = this.web3.toWei(ethAmount, 'ether');
return balanceWei;
}
public async getBalanceInEthAsync(owner: string): Promise<BigNumber.BigNumber> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this function? Wouldn't it be better if we only operate on wei?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense to me. This was a legacy method ported from OTC. I'll refactor it to return wei.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 95.712% when pulling e780664 on addWrappedETHSupport into d5ed586 on master.

…test assertions to check if the discrepancy between the existing ETH balance and expected balance is small enough to simply be the gas cost used by the transaction.
…o addWrappedETHSupport

# Conflicts:
#	test/ether_token_wrapper_test.ts
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 95.714% when pulling f15451e on addWrappedETHSupport into d5ed586 on master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants