Skip to content

Enable soft bundle in devnet and testnet#18302

Merged
mystenmark merged 7 commits intoMystenLabs:mainfrom
shio-coder:enable_soft_bundle
Jun 26, 2024
Merged

Enable soft bundle in devnet and testnet#18302
mystenmark merged 7 commits intoMystenLabs:mainfrom
shio-coder:enable_soft_bundle

Conversation

@shio-coder
Copy link
Copy Markdown
Contributor

@shio-coder shio-coder commented Jun 18, 2024 •
edited by ronny-mysten
Loading

Description

This is a follow up change after PR #17508 and #17868 , for SIP-19.

The change adds a unit test for soft bundle failure modes, as well as enable soft bundle feature flag in devnet and testnet.

Test plan

The feature is not exactly user-facing and do not break anything.

Release notes

  • Protocol: Enable soft bundle on Protocol Version 50 in Devnet and Testnet.
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

@shio-coder shio-coder requested a review from mystenmark as a code owner June 18, 2024 07:56
@vercel
Copy link
Copy Markdown

vercel bot commented Jun 18, 2024

@shio-coder is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 18, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2024 5:03am

@shio-coder shio-coder force-pushed the enable_soft_bundle branch from 6df1e65 to b3f4872 Compare June 19, 2024 03:14
mystenmark
mystenmark approved these changes Jun 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mystenmark mystenmark left a comment

Choose a reason for hiding this comment

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

This all looks good. We need one additional test, which is to make sure that bad certs in soft bundles are rejected. The easiest way to do this is most likely to add another check here

sui/crates/sui-core/src/unit_tests/transaction_tests.rs

Line 435 in c9b0432

.handle_certificate_v2(ct.clone(), Some(socket_addr))

which calls handle_soft_bundle_certificates_v3 in addition to handle_certificate_v2 and makes sure the error is the same. I think its okay to use a soft bundle of size one for this check.

@shio-coder shio-coder force-pushed the enable_soft_bundle branch from b3f4872 to 5b4682f Compare June 22, 2024 03:00
@shio-coder
Copy link
Copy Markdown
Contributor Author

shio-coder commented Jun 22, 2024

Thanks Mark! I have added forged cert tests for Soft Bundle.

It is a little bit different though because we have to make sure the tx has access to a shared object.
I have modified existing tests so that they run against two txs (one transfer, one move call w/ shared object).

@shio-coder shio-coder force-pushed the enable_soft_bundle branch from 5b4682f to fa15711 Compare June 25, 2024 05:33
lxfind
lxfind reviewed Jun 25, 2024
View reviewed changes
@shio-coder shio-coder force-pushed the enable_soft_bundle branch from fa15711 to 5b9ab84 Compare June 25, 2024 22:52
@shio-coder shio-coder requested a review from lxfind June 25, 2024 22:53
@shio-coder shio-coder force-pushed the enable_soft_bundle branch from 5b9ab84 to 1254b2e Compare June 26, 2024 00:18
@mystenmark mystenmark enabled auto-merge (squash) June 26, 2024 05:02
@mystenmark mystenmark merged commit 7f93f75 into MystenLabs:main Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lxfind lxfind lxfind approved these changes

@mystenmark mystenmark mystenmark approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shio-coder @lxfind @mystenmark