Add implementation for gas charging to address balance #23682
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
alex-mysten
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
lxfind
reviewed
Sep 23, 2025
crates/sui-types/src/transaction.rs
Outdated
| } | ||
|
|
||
| if self.gas_data().is_paid_from_address_balance() { | ||
| let gas_withdraw = FundsWithdrawalArg::balance_from_sender( |
There was a problem hiding this comment.
Depending on the gas owner, this may or may not be the sender
There was a problem hiding this comment.
Added support for different gas owner and associated e2e test cases
alex-mysten
force-pushed
the
steka-ab-gas-model
branch
from
c021999 to
7f9fb60
Compare
alex-mysten
force-pushed
the
steka-ab-gas-impl
branch
from
22324fc to
904f053
Compare
alex-mysten
force-pushed
the
steka-ab-gas-model
branch
from
7f9fb60 to
f962557
Compare
alex-mysten
requested review from
amnn,
emmazzz,
evan-wall-mysten,
henryachen,
tpham-mysten and
wlmyng
alex-mysten
force-pushed
the
steka-ab-gas-model
branch
3 times, most recently
from
980c32b to
67d0e7a
Compare
alex-mysten
force-pushed
the
steka-ab-gas-impl
branch
from
904f053 to
0a3cd7f
Compare
alex-mysten
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
alex-mysten
force-pushed
the
steka-ab-gas-impl
branch
from
0a3cd7f to
6e76eaa
Compare
alex-mysten
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mystenmark
reviewed
Oct 14, 2025
| ); | ||
|
|
||
| let delete_gas_summary = delete_effects.gas_cost_summary(); | ||
| let _delete_gas_used = calculate_total_gas_cost(delete_gas_summary); |
There was a problem hiding this comment.
were you trying to verify that there is a net gas rebate here? that does seem useful to test since there is an explicit branch in the code on +/- net gas
mystenmark
reviewed
Oct 14, 2025
| sender.hash(&mut hasher); | ||
| gas_owner.hash(&mut hasher); | ||
| "storage".hash(&mut hasher); | ||
| let nonce = hasher.finish() as u32; |
There was a problem hiding this comment.
why do we need the nonce? the sender and gas_owner will already be committed to by the tx digest?
mystenmark
reviewed
Oct 14, 2025
| let WithdrawFrom::Sender = withdraw.withdraw_from; | ||
| let account_address = match withdraw.withdraw_from { | ||
| WithdrawFrom::Sender => self.sender(), | ||
| WithdrawFrom::Sponsor => self.gas_owner(), |
There was a problem hiding this comment.
Should we ban explicit Sponsor withdrawals for now? Its a bit of an additional security burden on sponsor code that I'm not sure they are ready for.
alex-mysten
force-pushed
the
steka-ab-gas-impl
branch
from
6e76eaa to
5f12b4b
Compare
alex-mysten
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR charges gas to a transaction signer's account balance. Gated by protocol config; see referenced branch
Test plan
New e2e tests added
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.