ICTT Deployment Fixes #2227
Merged
ICTT Deployment Fixes #2227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arturrez
approved these changes
Oct 8, 2024
sukantoraymond
approved these changes
Oct 8, 2024
felipemadero
reviewed
Oct 8, 2024
| @@ -458,6 +459,7 @@ func TokenHomeAddCollateral( | |||
| remoteAddress common.Address, | |||
| amount *big.Int, | |||
| ) error { | |||
| ux.Logger.PrintToUser("Collateralizing remote contract on the home chain") | |||
There was a problem hiding this comment.
should we add action description logs to this lib? besides this, should we pass the logger as an arg?
| rpcURL string, | ||
| privateKey string, | ||
| remoteAddress common.Address, | ||
| ) error { | ||
| ux.Logger.PrintToUser("Registering remote contract with home contract") |
| // Get the decimal count for the token to format the balance. | ||
| // Note: decimals() is not officially part of the IERC20 interface, but is a common extension. | ||
| decimals, err := token.Decimals(nil) | ||
| if err != nil { |
There was a problem hiding this comment.
should we default to some standard decimal in case of error for not being official?
| homeKeyAddress = crypto.PubkeyToAddress(pk.PublicKey).Hex() | ||
| // Get the key to be used to deploy the token home contract and collateralize the remote | ||
| // in the case that it is a native token remote. | ||
| homeKey, homeKeyAddress, err := getHomeKeyAndAddress(app, network, flags.homeFlags) |
There was a problem hiding this comment.
previously the bug was not asking for it when needed, but now it may also ask for it when not needed,
as in the case the home preexists and there is no collateralization needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this should be merged
homeKeywas left uninitialized prior to use when using an existingTokenHomecontract and deploying a newNativeTokenRemoteTokenHomecontract was previously in the remote denomination. Now it uses the correct amount in the home denomination, as reported by the home contract.How this works
decimals()count ifuseGweiis false.How this was tested
Going through the flow of running
avalanche ictt deploy --deploy-native-remotefor a Fuji L1, and deploying aNativeTokenRemoteconnected to the existing USDCTokenHomecontract on the C-ChainHow is this documented
N/A