Skip to content

Fix gas payment: support tx.gas with address balance via coin reservation ref#930

Draft
hayes-mysten wants to merge 6 commits intomainfrom
fix-gas-payment-address-balance
Draft

Fix gas payment: support tx.gas with address balance via coin reservation ref#930
hayes-mysten wants to merge 6 commits intomainfrom
fix-gas-payment-address-balance

Conversation

@hayes-mysten
Copy link
Contributor

@hayes-mysten hayes-mysten commented Mar 6, 2026

Description

When a transaction uses Argument::GasCoin (tx.gas) — the pattern used by old-style SUI transfers (e.g. tx.splitCoins(tx.gas, [amount])) — and the sender's SUI is held entirely as address balance, setGasPayment previously threw "No valid gas coins found". This happened because:

  1. The address balance check was skipped when usesGasCoin = true
  2. The 2.0 SDK's listCoins filters fake/reservation coins from JSON-RPC responses, leaving no payment coins

This PR constructs a coin reservation reference (per sui/crates/sui-types/src/coin_reservation.rs) and includes it in the gas payment when usesGasCoin = true and the sender has address balance. The validator uses this special ObjectRef to draw gas from address balance.

The fake coin encodes:

  • objectId: XOR(accumulatorDynamicFieldId, chainIdentifier) — masked for cross-chain replay protection
  • version: "0" (unused by validation)
  • digest: [budget:u64le][epoch:u32le][0xac×20 magic]

Gas payment logic after this fix:

Scenario Payment
No tx.gas usage, AB covers budget + withdrawals []
No tx.gas usage, insufficient AB [realCoins…]
Uses tx.gas, sender has AB [realCoins…, fakeCoin]
Uses tx.gas, no AB [realCoins…]

Note: gRPC clients are unaffected — their resolveTransactionData delegates to server-side gas selection (doGasSelection=true) which already handles this.

Test plan

  • Unit test: mock client returning empty listCoins + nonzero addressBalance, build a tx with tx.gas, verify payment contains a fake coin with magic bytes in digest positions [12..32]
  • Regression: tx without tx.gas with sufficient AB still produces payment = []
  • E2E (when AB-enabled testnet is available): tx.splitCoins(tx.gas, [amount]) from an account with only address balance succeeds via JSON-RPC client

AI Assistance Notice

Please disclose the usage of AI. This is primarily to help inform reviewers of how careful they need to review PRs, and to keep track of AI usage across our team. Please fill this out accurately, and do not modify the content or heading for this section!

  • This PR was primarily written by AI.
  • I used AI for docs / tests, but manually wrote the source code.
  • I used AI to understand the problem space / repository.
  • I did not use AI for this PR.

hayes-mysten and others added 6 commits March 4, 2026 13:01
Previously setGasPayment only checked address balance for sponsored
transactions (when gasData.owner was set). This caused "No valid gas
coins found" errors for accounts with sufficient address balance but
no coin objects. Now address balance is checked for all gas payers,
addresses are normalized for comparison, and gasPayer is explicitly
validated.

Also skips JSON RPC variant of the address balance test due to a
server-side bug where executeTransactionBlock returns empty
balanceChanges for payment: [] transactions, and updates sui-tools
image.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When events: true is included, each event now contains a json field with the
JSON representation of the event's Move struct data (or null if unavailable).
Supported across all three transports (gRPC, GraphQL, JSON-RPC).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The sui keytool zk-login-insecure-sign-personal-message command's table
output was not reliably parsed by the regex in CI with the newer
sui-tools image. Switch to --json flag for deterministic JSON output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The new sui-tools Docker image requires a wallet config at the default
path for `sui keytool` commands. This commit:
- Creates default sui config in globalSetup.ts before tests run
- Adds `execKeytool` helper with isolated keystores and retry logic
  to handle concurrent keytool calls that can crash the sui binary
- Uses `parseKeytoolJson` to robustly extract JSON from keytool output
  that may contain debug lines before the JSON object
- Removes invalid `--client.config` flag from keytool calls (keytool
  only accepts `--keystore-path`)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The server-side bug causing empty balanceChanges for address-balance-backed
gas transactions was fixed in MystenLabs/sui#25685. Update the Docker image
to include the fix and remove the test skip.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a transaction uses Argument::GasCoin (e.g. tx.splitCoins(tx.gas, [amount]))
and the sender's SUI is held as address balance, setGasPayment now constructs a
coin reservation ObjectRef (encoded per coin_reservation.rs) and includes it in
the gas payment array. This allows the validator to draw gas from address balance
rather than requiring a coin object, fixing "No valid gas coins found" errors for
accounts whose SUI is entirely in address balance.

The fake coin encodes:
- objectId: XOR(accumulatorDynamicFieldId, chainIdentifier) for replay protection
- version: "0"
- digest: [budget:u64le][epoch:u32le][0xac...ac magic:20]

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 6, 2026

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

Project Deployment Actions Updated (UTC)
sui-typescript-docs Ready Ready Preview, Comment Mar 6, 2026 8:54pm

Request Review

Copy link
Contributor

@clud-bot clud-bot bot left a comment

Choose a reason for hiding this comment

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

Blocking issue in packages/sui/src/client/core-resolver.ts:

This change injects a coin-reservation-style ObjectRef into gasData.payment when usesGasCoin && addressBalance > 0. On the Sui side (including the sui-tools commit this PR pins: e50c3fe5...), transaction validity still rejects reservation digests in gas payment objects (ParsedDigest::try_from(gas_digest).is_err() in crates/sui-types/src/transaction.rs), and coin_reservation_obj_refs still only scans PT inputs (with a TODO to add gas coin obj refs).

So the new path appears to generate gas payments that will be rejected at signing/validation time, rather than fixing execution for tx.gas + address-balance-only accounts.

Please either gate this behavior on protocol support, or update the implementation to match the currently-supported path in Sui.

@hayes-mysten hayes-mysten marked this pull request as ready for review March 10, 2026 21:41
@hayes-mysten hayes-mysten requested a review from a team as a code owner March 10, 2026 21:41
@hayes-mysten hayes-mysten marked this pull request as draft March 10, 2026 21:44
@Danny-Devs
Copy link
Contributor

Looking at the addressBalance > 0n guard — check_amounts_available() in funds_read.rs hard-rejects when the reservation amount exceeds the actual balance (InvalidWithdrawReservation). Would it make sense to guard with addressBalance >= budget here so users get a clear SDK error instead of the validator rejection?

@hayes-mysten
Copy link
Contributor Author

yeah, this whole PR is llm slop and won't be merged as is. We are still talking about how to handle tx.gas and trying to figure out exactly what that will look like in different cases.

Mostly just kicked this off in claude to see what one option might look like, but I'll be opening a new PR with a proper implementation once we land on a solution.

The build vs validator error question is an interesting one, and there are definitely tradeoffs. My personal take is that if the transaction could never be valid its better to error at build time, but for balances (especially with address balances) building should still be allowed with an insufficient balance. We may want an option for this, but balances can change frequently, and not being able to pre-build for a transaction you can't currently fund probably still makes sense.

I think it should be up to apps or wallets to validate balances, but definitely open to other thoughts on this

@Danny-Devs
Copy link
Contributor

Danny-Devs commented Mar 13, 2026

That tracks — Claude checked how the other major SDKs handle this and it appears that they all draw the same line:

SDK Balance check at build? Where errors surface
ethers.js v6 No Parses node error strings into typed errors
viem No InsufficientFundsError wraps node response
@solana/web3.js No RPC preflight simulation or validator
cosmjs No Chain broadcast rejection

Structural invalidity → build error.
Balance insufficiency → deferred to execution.

One thought: would a high-level preflight helper (e.g. validateGasBalance) be useful?

Right now, underfunded transactions surface as errors like InsufficientGas,
InvalidWithdrawReservation, or "No valid gas coins found" — none of which
tell the caller how much they actually need vs. have.

A helper that simulates execution and surfaces the gap could look like:

const tx = buildMyTransaction();           // build (no balance check)

await validateGasBalance(tx, signer);      // opt-in:
                                           // "You need 0.05 SUI, you have 0.02"

await client.signAndExecute(tx);           // send

Not baked into the build path — just a tool on the shelf for wallets that want it.

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.

2 participants