feat(cosmwasm): load-time validation for instantiated contract addresses#1309
Open
AttissNgo wants to merge 1 commit into
Open
feat(cosmwasm): load-time validation for instantiated contract addresses#1309AttissNgo wants to merge 1 commit into
AttissNgo wants to merge 1 commit into
Conversation
- add InstantiatedContractConfig interface with required address field - add load-time validation for Amplifier Protocol contracts - add new getter methods - replace awkward validateRequired pattern
| this.chains = fullConfig.chains; | ||
|
|
||
| this.validateConfig(); | ||
| this.validateAmplifierProtocolContracts(); |
Contributor
There was a problem hiding this comment.
This method should be part of the validateConfig. Note that currently we print warnings instead of errors, since axelarate devnet setup workflow does not produce a full valid config. The validation methods gathers all the errors and then produce a report instead
| printWarn(''); | ||
| } | ||
|
|
||
| private validateAmplifierProtocolContracts(): void { |
Contributor
There was a problem hiding this comment.
As mentioned above, this method should be used as a part of validateConfig. It should return errors that are relevant to its validation checks
| } | ||
|
|
||
| public getContractAddress(name: string): string { | ||
| const config = this.getContractConfig(name); |
Contributor
There was a problem hiding this comment.
Suggested change
| const config = this.getContractConfig(name); | |
| const config = this.getInstantiatedContractConfig(name); |
To be really certain about the address presence
| const itsMsgTranslator = | ||
| chainParams?.msgTranslator || | ||
| config.validateRequired(config.getContractConfig('ItsAbiTranslator').address, 'ItsAbiTranslator.address'); | ||
| const itsMsgTranslator = chainParams?.msgTranslator || config.getInstantiatedContractConfig('ItsAbiTranslator').address; |
Contributor
There was a problem hiding this comment.
Suggested change
| const itsMsgTranslator = chainParams?.msgTranslator || config.getInstantiatedContractConfig('ItsAbiTranslator').address; | |
| const itsMsgTranslator = chainParams?.msgTranslator || config.getContractAddress('ItsAbiTranslator'); |
| } | ||
|
|
||
| const contractAddress = config.validateRequired(config.getContractConfig(contractName).address, `${contractName}.address`); | ||
| const contractAddress = config.getInstantiatedContractConfig(contractName).address; |
Contributor
There was a problem hiding this comment.
Suggested change
| const contractAddress = config.getInstantiatedContractConfig(contractName).address; | |
| const contractAddress = config.getContractAddress(contractName); |
| const chainCodec = this.getChainCodecContractForChainType(chainType); | ||
| const chainCodecConfig = this.getContractConfig(chainCodec); | ||
| return this.validateRequired(chainCodecConfig.address, `${chainCodec}.address`); | ||
| return this.getInstantiatedContractConfig(chainCodec).address; |
Contributor
There was a problem hiding this comment.
Suggested change
| return this.getInstantiatedContractConfig(chainCodec).address; | |
| return this.getContractAddress(chainCodec); |
Comment on lines
+82
to
+83
| const contractName = config.getVotingVerifierContractForChainType(chainConfig.chainType); | ||
| const address = config.getInstantiatedContractByChain(contractName, chainName).address; |
Contributor
There was a problem hiding this comment.
nit: This change complicates that part, maybe leave the way it was
Comment on lines
+456
to
+458
| if (!config.address) { | ||
| throw new Error(`Contract '${name}' has not been instantiated`); | ||
| } |
Contributor
There was a problem hiding this comment.
We can use the existing validation method, also used in this file
Comment on lines
+464
to
+466
| if (!config.address) { | ||
| throw new Error(`Contract '${name}' for chain '${chainName}' has not been instantiated`); | ||
| } |
Contributor
There was a problem hiding this comment.
We can use the existing validation method, also used in this file
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Improve type safety and provide fail-fast validation with clear error messages when required contracts are missing addresses
Note
Adds load-time validation for required contract addresses, introduces typed getters for instantiated contracts, and refactors CosmWasm commands/migrations/queries to use them with chain-type-aware resolution.
InstantiatedContractConfig,AMPLIFIER_PROTOCOL_CONTRACTS, andvalidateAmplifierProtocolContracts()to enforce required addresses at load.getContractAddress,getInstantiatedContractConfig,getInstantiatedContractByChain; use ingetChainCodecAddress().validateRequired(...address)with new getters acrosscontract.ts,coordinator.ts,migrate/chain-codec.ts,migrate/sdk50.ts,migrate/update-rewards-pool-epoch-duration.ts,query.ts, andutils.js(e.g.,itsHubChainParams).VotingVerifier/MultisigProverduring operations and migrations.Written by Cursor Bugbot for commit 7a895db. This will update automatically on new commits. Configure here.