Skip to content

gas data tests across execution entry points#26179

Merged
dariorussi merged 1 commit intomainfrom
dario/gas_tests
Apr 13, 2026
Merged

gas data tests across execution entry points#26179
dariorussi merged 1 commit intomainfrom
dario/gas_tests

Conversation

@dariorussi
Copy link
Copy Markdown
Contributor

@dariorussi dariorussi commented Apr 9, 2026

Description

We check that bad gas data produces expected and consistent results across entry point.
Bogus data for address balance and free transaction is not really exercised yet and should come with a later PR (@mystenmark @alex-mysten @aschran if you have suggestions)

Co-Authored-By: My Dog (1M bones) <noreply@my_dog.com> and Claude Opus 4.6 (1M context) noreply@anthropic.com

Test plan

This is it


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.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • Indexing Framework:

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sui-docs Ready Ready Preview, Comment Apr 13, 2026 5:09pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
multisig-toolkit Ignored Ignored Preview Apr 13, 2026 5:09pm
sui-kiosk Ignored Ignored Preview Apr 13, 2026 5:09pm

Request Review

Copy link
Copy Markdown
Contributor

@tzakian tzakian left a comment

Choose a reason for hiding this comment

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

One question on a test but otherwise LGTM

}
_ => unreachable!(),
};
println!("{entry_point}: {result:?}");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should there be an assertion on the result here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

well definitely there should NOT be a println

Copy link
Copy Markdown
Contributor

@alex-mysten alex-mysten left a comment

Choose a reason for hiding this comment

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

This looks good to me - one comment - happy to add the address balance / gasless variants once merged

// =============================================================================

#[tokio::test]
async fn test_simple_transfer_gas_all_paths() {
Copy link
Copy Markdown
Contributor

@alex-mysten alex-mysten Apr 10, 2026

Choose a reason for hiding this comment

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

what about a valid gas coin with insufficient balance for gas_budget? thinking about how that would translate to address balances too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we should have those tests elsewhere, this was mostly to track bad gas data and transaction data related to gas.
To be honest I would like to review all gas tests because we had few issues and I believe we can organize them better

@dariorussi dariorussi temporarily deployed to sui-typescript-aws-kms-test-env April 13, 2026 16:25 — with GitHub Actions Inactive
@dariorussi
Copy link
Copy Markdown
Contributor Author

addressed feedback

@dariorussi dariorussi temporarily deployed to sui-typescript-aws-kms-test-env April 13, 2026 17:07 — with GitHub Actions Inactive
@dariorussi dariorussi merged commit 0ffa28e into main Apr 13, 2026
65 of 68 checks passed
@dariorussi dariorussi deleted the dario/gas_tests branch April 13, 2026 17:39
jordanjennings-mysten pushed a commit that referenced this pull request Apr 16, 2026
## Description 

We check that bad gas data produces expected and consistent results
across entry point.
Bogus data for address balance and free transaction is not really
exercised yet and should come with a later PR (@mystenmark @alex-mysten
@aschran if you have suggestions)

Co-Authored-By: My Dog (1M bones) <noreply@my_dog.com> and Claude Opus
4.6 (1M context) <noreply@anthropic.com>

## Test plan 

This is it

---

## 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.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] gRPC:
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] Indexing Framework:
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.

3 participants