Skip to content
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

feat: add stellar upgrader contract to the deploy-contract.js script #487

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cgorenflo
Copy link

@cgorenflo cgorenflo commented Dec 24, 2024

AXE-6746

@cgorenflo cgorenflo requested a review from a team as a code owner December 24, 2024 00:19
stellar/deploy-contract.js Show resolved Hide resolved
stellar/deploy-contract.js Outdated Show resolved Hide resolved
await broadcast(operation, wallet, chain, 'Upgraded contract', options);
chain.contracts[contractName].wasmHash = serializeValue(newWasmHash);
printInfo('Contract upgraded successfully!', contractAddress);
}

async function createUpgradeAuths(contractAddress, newWasmHash, migrationData, chain, wallet) {
// 20 seems a reasonable number of ledgers to allow for the upgrade to take effect
Copy link
Member

Choose a reason for hiding this comment

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

Create a global constant for it. Could also create an option with a default of 20 though I don't see us configuring it

Copy link
Author

Choose a reason for hiding this comment

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

you mean for the 20? What would be the benefit of that? It's literally just used here once, and, as you point out, that doesn't seem like something we would ever parameterize

Copy link
Member

Choose a reason for hiding this comment

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

yeah, parameterization seems overkill. Would still be nice to define a constant variable for it though for style

stellar/deploy-contract.js Outdated Show resolved Hide resolved
node stellar/deploy-contract.js upgrade <CONTRACT_NAME> --chain-name <CHAIN_NAME> --wasm-path ../axelar-cgp-soroban/target/wasm32-unknown-unknown/release/<CONTRACT_NAME>.optimized.wasm --new-version <NEW_VERSION> --migration-data <MIGRATION_DATA>
```

where `<CONTRACT_NAME>` is the name of the contract to be upgraded and `--wasm-path` points to the upgraded bytecode. As a sanity check, `<NEW_VERSION>` must match the version number defined by the provided bytecode, so upgrading to the wrong version can be prevented. `<MIGRATION_DATA>` is the json encoded data that will be passed to the contract's `migrate` function.
Copy link
Member

Choose a reason for hiding this comment

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

Can you mention how MIGRATION_DATA is encoded for the no migration case and a more complex migration case?

await broadcast(operation, wallet, chain, 'Upgraded contract', options);
chain.contracts[contractName].wasmHash = serializeValue(newWasmHash);
printInfo('Contract upgraded successfully!', contractAddress);
}

async function createUpgradeAuths(contractAddress, newWasmHash, migrationData, chain, wallet) {
// 20 seems a reasonable number of ledgers to allow for the upgrade to take effect
Copy link
Member

Choose a reason for hiding this comment

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

yeah, parameterization seems overkill. Would still be nice to define a constant variable for it though for style

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

Successfully merging this pull request may close these issues.

2 participants