-
Notifications
You must be signed in to change notification settings - Fork 30
feat(cosmwasm): load-time validation for instantiated contract addresses #1309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -67,6 +67,10 @@ export interface ContractConfig { | |||||
| lastUploadedCodeId?: number; | ||||||
| } | ||||||
|
|
||||||
| export interface InstantiatedContractConfig extends ContractConfig { | ||||||
| address: string; | ||||||
| } | ||||||
|
|
||||||
| export interface ContractsChainInfo { | ||||||
| address: string; | ||||||
| codeId: number; | ||||||
|
|
@@ -125,6 +129,8 @@ export interface GatewayChainConfig { | |||||
| address?: string; | ||||||
| } | ||||||
|
|
||||||
| const AMPLIFIER_PROTOCOL_CONTRACTS = ['ServiceRegistry', 'Router', 'Coordinator', 'Multisig', 'Rewards'] as const; | ||||||
|
|
||||||
| export class ConfigManager implements FullConfig { | ||||||
| private environment: string; | ||||||
|
|
||||||
|
|
@@ -146,6 +152,7 @@ export class ConfigManager implements FullConfig { | |||||
| this.chains = fullConfig.chains; | ||||||
|
|
||||||
| this.validateConfig(); | ||||||
| this.validateAmplifierProtocolContracts(); | ||||||
| } | ||||||
|
|
||||||
| private validateConfig(): void { | ||||||
|
|
@@ -333,6 +340,21 @@ export class ConfigManager implements FullConfig { | |||||
| printWarn(''); | ||||||
| } | ||||||
|
|
||||||
| private validateAmplifierProtocolContracts(): void { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above, this method should be used as a part of |
||||||
| const missing: string[] = []; | ||||||
|
|
||||||
| for (const contractName of AMPLIFIER_PROTOCOL_CONTRACTS) { | ||||||
| const config = this.getContractConfig(contractName); | ||||||
| if (!config.address) { | ||||||
| missing.push(contractName); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (missing.length > 0) { | ||||||
| throw new Error(`Amplifier Protocol contracts missing addresses in ${this.environment} config: ${missing.join(', ')}.`); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private isValidGasPrice(price: string): boolean { | ||||||
| const numericOnlyPattern = /^\d+$/; | ||||||
| const withDenominationPattern = /^\d+(\.\d+)?[a-zA-Z]+$/; | ||||||
|
|
@@ -424,6 +446,27 @@ export class ConfigManager implements FullConfig { | |||||
| return contractConfig[chainName]; | ||||||
| } | ||||||
|
|
||||||
| public getContractAddress(name: string): string { | ||||||
| const config = this.getContractConfig(name); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
To be really certain about the address presence |
||||||
| return config.address!; | ||||||
| } | ||||||
|
|
||||||
| public getInstantiatedContractConfig(name: string): InstantiatedContractConfig { | ||||||
| const config = this.getContractConfig(name); | ||||||
| if (!config.address) { | ||||||
| throw new Error(`Contract '${name}' has not been instantiated`); | ||||||
| } | ||||||
|
Comment on lines
+456
to
+458
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use the existing validation method, also used in this file |
||||||
| return config as InstantiatedContractConfig; | ||||||
| } | ||||||
|
|
||||||
| public getInstantiatedContractByChain(name: string, chainName: string): InstantiatedContractConfig { | ||||||
| const config = this.getContractConfigByChain(name, chainName); | ||||||
| if (!config.address) { | ||||||
| throw new Error(`Contract '${name}' for chain '${chainName}' has not been instantiated`); | ||||||
| } | ||||||
|
Comment on lines
+464
to
+466
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use the existing validation method, also used in this file |
||||||
| return config as InstantiatedContractConfig; | ||||||
| } | ||||||
|
|
||||||
| public validateRequired<T>(value: T | undefined | null, configPath: string, type?: string): T { | ||||||
| if (value === undefined || value === null || (typeof value === 'string' && value.trim() === '')) { | ||||||
| throw new Error(`Missing required configuration for the chain. Please configure it in ${configPath}.`); | ||||||
|
|
@@ -535,8 +578,7 @@ export class ConfigManager implements FullConfig { | |||||
|
|
||||||
| public getChainCodecAddress(chainType: string): string { | ||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| public getVotingVerifierContract(chainName: string): VotingVerifierChainConfig { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -62,7 +62,7 @@ const executeContractMessage = async ( | |||||
| throw new Error('At least one message is required'); | ||||||
| } | ||||||
|
|
||||||
| const contractAddress = config.validateRequired(config.getContractConfig(contractName).address, `${contractName}.address`); | ||||||
| const contractAddress = config.getInstantiatedContractConfig(contractName).address; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| if (options.governance) { | ||||||
| validateGovernanceMode(config, contractName, chainName); | ||||||
|
|
@@ -138,9 +138,9 @@ const registerProtocol = async ( | |||||
| _args?: string[], | ||||||
| fee?: string | StdFee, | ||||||
| ): Promise<void> => { | ||||||
| const serviceRegistry = config.validateRequired(config.getContractConfig('ServiceRegistry').address, 'ServiceRegistry.address'); | ||||||
| const router = config.validateRequired(config.getContractConfig('Router').address, 'Router.address'); | ||||||
| const multisig = config.validateRequired(config.getContractConfig('Multisig').address, 'Multisig.address'); | ||||||
| const serviceRegistry = config.getContractAddress('ServiceRegistry'); | ||||||
| const router = config.getContractAddress('Router'); | ||||||
| const multisig = config.getContractAddress('Multisig'); | ||||||
|
|
||||||
| const msg = [ | ||||||
| { | ||||||
|
|
@@ -186,11 +186,10 @@ const createRewardPools = async ( | |||||
|
|
||||||
| const threshold: string[] = config.parseThreshold(participationThreshold, '--participationThreshold'); | ||||||
|
|
||||||
| const votingVerifierAddress = config.validateRequired( | ||||||
| config.getVotingVerifierContract(chainName).address, | ||||||
| `VotingVerifier[${chainName}].address`, | ||||||
| ); | ||||||
| const multisigAddress = config.validateRequired(config.getContractConfig('Multisig').address, 'Multisig.address'); | ||||||
| const chainConfig = config.getChainConfig(chainName); | ||||||
| const verifierContractName = config.getVotingVerifierContractForChainType(chainConfig.chainType); | ||||||
| const votingVerifierAddress = config.getInstantiatedContractByChain(verifierContractName, chainName).address; | ||||||
| const multisigAddress = config.getContractAddress('Multisig'); | ||||||
|
|
||||||
| const messages = [ | ||||||
| { | ||||||
|
|
@@ -245,7 +244,7 @@ const routerFreezeChain = async ( | |||||
| throw new Error('Router freeze_chain can only be executed by Admin EOA, not via governance'); | ||||||
| } | ||||||
|
|
||||||
| const contractAddress = config.validateRequired(config.getContractConfig('Router').address, 'Router.address'); | ||||||
| const contractAddress = config.getContractAddress('Router'); | ||||||
| printDirectExecutionInfo(msg, contractAddress); | ||||||
| return executeDirectly(client, contractAddress, msg, fee); | ||||||
| }; | ||||||
|
|
@@ -265,7 +264,7 @@ const routerUnfreezeChain = async ( | |||||
| throw new Error('Router unfreeze_chain can only be executed by Admin EOA, not via governance'); | ||||||
| } | ||||||
|
|
||||||
| const contractAddress = config.validateRequired(config.getContractConfig('Router').address, 'Router.address'); | ||||||
| const contractAddress = config.getContractAddress('Router'); | ||||||
| printDirectExecutionInfo(msg, contractAddress); | ||||||
| return executeDirectly(client, contractAddress, msg, fee); | ||||||
| }; | ||||||
|
|
@@ -283,7 +282,7 @@ const routerDisableRouting = async ( | |||||
| throw new Error('Router disable_routing can only be executed by Admin EOA, not via governance'); | ||||||
| } | ||||||
|
|
||||||
| const contractAddress = config.validateRequired(config.getContractConfig('Router').address, 'Router.address'); | ||||||
| const contractAddress = config.getContractAddress('Router'); | ||||||
| printDirectExecutionInfo(msg, contractAddress); | ||||||
| return executeDirectly(client, contractAddress, msg, fee); | ||||||
| }; | ||||||
|
|
@@ -301,7 +300,7 @@ const routerEnableRouting = async ( | |||||
| throw new Error('Router enable_routing can only be executed by Admin EOA, not via governance'); | ||||||
| } | ||||||
|
|
||||||
| const contractAddress = config.validateRequired(config.getContractConfig('Router').address, 'Router.address'); | ||||||
| const contractAddress = config.getContractAddress('Router'); | ||||||
| printDirectExecutionInfo(msg, contractAddress); | ||||||
| return executeDirectly(client, contractAddress, msg, fee); | ||||||
| }; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,16 +29,15 @@ async function migrateAllVotingVerifiers( | |
| const votingVerifiers: Array<{ chainName: string; address: string; contractName: string }> = []; | ||
|
|
||
| for (const { name: chainName, config: chainConfig } of chains) { | ||
| const votingVerifierConfig = config.getVotingVerifierContract(chainName); | ||
| const contractName = config.getVotingVerifierContractForChainType(chainConfig.chainType); | ||
| config.validateRequired(votingVerifierConfig.address, 'votingVerifierConfig.address'); | ||
| const address = config.getInstantiatedContractByChain(contractName, chainName).address; | ||
|
|
||
| votingVerifiers.push({ | ||
| chainName, | ||
| address: votingVerifierConfig.address, | ||
| address, | ||
| contractName, | ||
| }); | ||
| printInfo(`Added ${chainName} voting verifier (address: ${votingVerifierConfig.address})`); | ||
| printInfo(`Added ${chainName} voting verifier (address: ${address})`); | ||
| } | ||
|
|
||
| printInfo(`Found ${votingVerifiers.length} voting verifier(s) to migrate`); | ||
|
|
@@ -80,9 +79,10 @@ async function updateBlockTimeRelatedParameters( | |
| chains.map(async ({ name: chainName, config: chainConfig }) => { | ||
| try { | ||
| const votingVerifierConfig = config.getVotingVerifierContract(chainName); | ||
| config.validateRequired(votingVerifierConfig.address, 'votingVerifierConfig.address'); | ||
| const contractName = config.getVotingVerifierContractForChainType(chainConfig.chainType); | ||
| const address = config.getInstantiatedContractByChain(contractName, chainName).address; | ||
|
Comment on lines
+82
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This change complicates that part, maybe leave the way it was |
||
|
|
||
| const { block_expiry } = await client.queryContractSmart(votingVerifierConfig.address, 'voting_parameters'); | ||
| const { block_expiry } = await client.queryContractSmart(address, 'voting_parameters'); | ||
|
|
||
| const message = { | ||
| update_voting_parameters: { | ||
|
|
@@ -99,12 +99,10 @@ async function updateBlockTimeRelatedParameters( | |
| `Current voting parameters for ${chainName}: block_expiry: ${block_expiry}. New proposed block_expiry: ${message.update_voting_parameters.block_expiry}`, | ||
| ); | ||
|
|
||
| const contractName = config.getVotingVerifierContractForChainType(chainConfig.chainType); | ||
|
|
||
| return { | ||
| chainName, | ||
| contractName, | ||
| address: votingVerifierConfig.address, | ||
| address, | ||
| message, | ||
| }; | ||
| } catch (error) { | ||
|
|
@@ -147,10 +145,10 @@ async function updateSigningParametersForMultisig( | |
| fee: string | StdFee, | ||
| ): Promise<void> { | ||
| const multisigConfig = config.getContractConfig('Multisig'); | ||
| config.validateRequired(multisigConfig.address, 'axelar.contracts.Multisig.address', 'string'); | ||
| const multisigAddress = config.getContractAddress('Multisig'); | ||
| config.validateRequired(multisigConfig.blockExpiry, 'axelar.contracts.Multisig.blockExpiry', 'number'); | ||
|
|
||
| const { block_expiry } = await client.queryContractSmart(multisigConfig.address, 'signing_parameters'); | ||
| const { block_expiry } = await client.queryContractSmart(multisigAddress, 'signing_parameters'); | ||
| printInfo(`Current signing parameters: block_expiry: ${block_expiry}. New proposed block_expiry: ${multisigConfig.blockExpiry}`); | ||
|
|
||
| const msg = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -915,9 +915,7 @@ const itsHubChainParams = (config, chainConfig) => { | |||||
|
|
||||||
| const key = chainConfig.axelarId.toLowerCase(); | ||||||
| const chainParams = config.axelar.contracts.InterchainTokenService[key]; | ||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| return { | ||||||
| itsEdgeContractAddress, | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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