-
Notifications
You must be signed in to change notification settings - Fork 690
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
Set Sui package manifest to pass source verification #3731
base: sui/testnet
Are you sure you want to change the base?
Set Sui package manifest to pass source verification #3731
Conversation
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.
Source verification seems like a positive step, but I'd want to make sure all of these fields are set correctly. There was also recent work on updating the dependencies in #3803 so perhaps this could be revisited after that is merged and the sui/mainnet
and sui/testnet
branches are updated similarly.
Testing branches for those are here:
https://github.com/wormhole-foundation/wormhole/tree/sui-upgrade-mainnet
https://github.com/wormhole-foundation/wormhole/tree/sui-upgrade-testnet
|
||
dependencies = [ | ||
{ name = "Sui" }, | ||
] | ||
|
||
[[move.package]] | ||
name = "MoveStdlib" | ||
source = { git = "https://github.com/MystenLabs/sui.git", rev = "09b2081498366df936abae26eea4b2d5cafb2788", subdir = "crates/sui-framework/packages/move-stdlib" } | ||
source = { git = "https://github.com/MystenLabs/sui.git", rev = "framework/testnet", subdir = "crates/sui-framework/packages/move-stdlib" } |
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.
might be best to keep these dependencies pinned to a commit hash
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.
We have to take into account that Sui
(along with MoveStdlib
at "0x1"
AFAIK) is a special package which can be upgraded onchain 'in-place'
(unlike regular Move package upgrades on Sui that in effect create a new
package with a new address.)
The advantage of using framework/testnet
is that the compiler will
automatically fetch the Sui
package that's currently on testnet, hence
source verification will succeed.
$ sui client verify-source --verify-deps
UPDATING GIT DEPENDENCY https://github.com/MystenLabs/sui.git
INCLUDING DEPENDENCY Sui
INCLUDING DEPENDENCY MoveStdlib
BUILDING Wormhole
// ...
Source verification succeeded!
However, let's say we published the package in the past and set Sui
to testnet-v1.17.1
in the manifest. Verification could fail in the
future to to upgrades to the Sui
package.
$ sui client verify-source --verify-deps
FETCHING GIT DEPENDENCY https://github.com/MystenLabs/sui.git
INCLUDING DEPENDENCY Sui
INCLUDING DEPENDENCY MoveStdlib
BUILDING Wormhole
// ...
Multiple source verification errors found:
- Local dependency did not match its on-chain version at 0000000000000000000000000000000000000000000000000000000000000002::Sui::bls12381
- Local version of dependency 0000000000000000000000000000000000000000000000000000000000000002::group_ops was not found.
For all other packages, it definitely makes sense to pin them to a
commit/tag.
@@ -1,14 +1,12 @@ | |||
[package] | |||
name = "Wormhole" | |||
version = "0.2.0" | |||
published-at = "0xcc029e2810f17f9f43f52262f40026a71fbdca40ed3803ad2884994361910b7e" |
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.
The address I see in the testnet branch is 0xf47329f4344f3bf0f8e436e2f7b485466cff300f12a166563995d3888c296a94
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.
That's interesting. Indeed, I changed published-at
and wormhole
to
0xf47329f4344f3bf0f8e436e2f7b485466cff300f12a166563995d3888c296a94
in Move.toml
and source verification passed too. Begs the question of
why there are two different deployments with the exact same source code.
In fact, at Aftermath we've been using 0xcc029e2810f17f9f43f52262f40026a71fbdca40ed3803ad2884994361910b7e
to verify Wormhole messages (and there's more activity in the contract
than just ours looking at the txs.)
Would be good to clarify what's the official version.
Update the Sui package manifest (
Move.toml
) so that it passes source verification against the package published on Sui's testnet.This ensures that downstream Sui Move packages that have this version of the
Wormhole
package as a dependency can verify that exact same code is published on the Sui Testnet viasui client verify-source --skip-source --verify-deps
.Instructions for verifying the source code have been added to the README.
If the PR is approved, it would be nice to tag the resulting commit so that other Sui Move packages can declare this as a dependency as follows.
I have ensured that the package still builds and tests pass: