-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
[sui-adapter] Resolve Move abort locations to package ID instead of runtime ID #17884
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
@@ -13,7 +13,7 @@ use tracing::{info, warn}; | |||
|
|||
/// The minimum and maximum protocol versions supported by this build. | |||
const MIN_PROTOCOL_VERSION: u64 = 1; | |||
const MAX_PROTOCOL_VERSION: u64 = 46; | |||
const MAX_PROTOCOL_VERSION: u64 = 47; |
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.
With the cut tomorrow, was surprised you needed to do this, but makes a bit of sense I suppose looking at what was enabled in 46
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.
Yea, was likewise surprised that there wasn't a protocol version sitting already for the cut. But yea, we definitely need to protocol config this change since it will change transaction effects.
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.
Of course there is now a merge conflict on another PR that landed that added 47... 🎉
5584112
to
af0cfa6
Compare
af0cfa6
to
8a03b18
Compare
Description
Updates Move aborts so that the address in the abort location uses the package ID as opposed to the runtime module ID.
Basically, before if you had a package
P
with a moduleM
published at
0xA
for version 1,0xB
for version 2, and0xC
for version 3, then before this changeAfter this change
The only meaningful changes are in
sui-execution/latest/sui-adapter/src/error.rs
the other changes are just plumbing/protocol config updates and tests.Test plan
Added tests to make sure existing behavior is preserved, and that new behavior works as expected.
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.