-
Notifications
You must be signed in to change notification settings - Fork 8
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(axelar_gateway)!: upgrade versioned #225
base: main
Are you sure you want to change the base?
Conversation
…in-transfer-event
…in-transfer-event
…in-transfer-event
…in-transfer-event
This reverts commit 94dac9a.
…in-transfer-event
…r_gateway_event_checking
Code Coverage SummaryClick to see the summary
Click to see the extended report
|
@@ -174,6 +175,19 @@ entry fun rotate_signers( | |||
) | |||
} | |||
|
|||
/// This function should only be called once |
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.
if this is called, it'll still fail due to the type mismatch since Gateway_v0 isn't stored anymore, right?
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.
you could call in many times in the same version, but it would still fail as there are checks
/// This function should only be called once | ||
/// (checks should be made on versioned to ensure this) | ||
/// It upgrades the version control to the new version control. | ||
entry fun migrate(self: &mut Gateway) { |
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.
entry fun migrate(self: &mut Gateway) { | |
entry fun migrate(self: &mut Gateway, _: &OwnerCap) { |
make it only callable by the owner
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.
Since this is for testing I wanted to keep it simple, and this will break the deployment scripts, want me to do it anyway?
/// It upgrades the version control to the new version control. | ||
entry fun migrate(self: &mut Gateway) { | ||
let (v0, cap) = self.inner.remove_value_for_upgrade<Gateway_v0>(); | ||
let v1 = v0.migrate(version_control()); |
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.
make the migration more interesting. e.g. add some message approval, and maybe another field to the gateway object, and add a check post upgrade?
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.
also from the doc,
For the second upgrade, also include a change to a dependency contract, such as Utils or so. The deployment scripts should handle upgrading the dependency as well on a change, instead of reusing the existing config address. Can we also include upgrading the example contract to point to the new gateway contract?
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.
what is "some message approval"? Messages can be approved already. Can you be more specific? What check post upgrade?
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.
since tests for allowed functions cannot be actually automated, I would have to write a custom function that lives on deployments that does nothing for our actual gateway to be able to test this 'new' field, how do you propose we go about this?
@@ -276,6 +290,7 @@ public fun take_approved_message( | |||
|
|||
fun version_control(): VersionControl { | |||
version_control::new(vector[ | |||
vector[], |
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.
keep some functions available to test different cases
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.
cannot
No description provided.