Refactor and renames #61
Conversation
yorhodes
force-pushed
the
config-simplification
branch
from
9a095c8 to
9c6819a
Compare
Hardhat Coverage ReportCoverage after merging config-simplification into main will be
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||
|
|
||
| const [name, symbol, totalSupply, decimals] = await Promise.all([ | ||
| erc20.name(), |
There was a problem hiding this comment.
Nit, can we reduce nesting here?
| * @author Abacus Works | ||
| * @dev Supply on each chain is not constant but the aggregate supply across all chains is. | ||
| */ | ||
| contract HypNative is TokenRouter { | ||
| contract HypNativeCollateral is TokenRouter { |
| async transfer( | ||
| origin: ChainName, | ||
| destination: ChainName, | ||
| recipient: types.Address, | ||
| amount: BigNumberish, | ||
| ) { | ||
| const originRouter = this.getContracts(origin).router; | ||
| const originRouter = this.router(this.getContracts(origin)); |
There was a problem hiding this comment.
I would really like us to detect the contract type and:
- approve ERC20Collateral
- Set msg.value if NativeCollateral
There was a problem hiding this comment.
Probably best for a follow up PR that covers what we discussed yesterday (i.e. passing chain/tokenType to the app constructor)
There was a problem hiding this comment.
Another request I'd love to have in that PR is a getBalance(chain) function that will give you the balance of the native asset, collateral asset, or wrapped asset, depending on the chain
|
|
||
| async transfer( | ||
| origin: ChainName, | ||
| destination: ChainName, | ||
| recipient: types.Address, | ||
| amountOrId: BigNumberish, | ||
| ) { | ||
| const originRouter = this.getContracts(origin).router; | ||
| const originRouter = this.router(this.getContracts(origin)); |
There was a problem hiding this comment.
This fn does not support native collateral, since value should be gasPayment + amountOrId
| export type NativeConfig = { | ||
| type: TokenType.native; | ||
| type: TokenType.collateral | TokenType.collateralUri | TokenType.native; | ||
| token?: string; // no token implies native collateral |
There was a problem hiding this comment.
nit: Maybe token not required for native collateral
Just b/c the type is not implied by the presence/absence of token, it's explicit in the type
HypNative=>HypNativeCollateral