-
Notifications
You must be signed in to change notification settings - Fork 29
fix(server): use address balances for Walrus uploads #527
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
Changes from 3 commits
f94997c
8849821
efc3042
4bf0de1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import type { Transaction } from "@mysten/sui/transactions"; | ||
| import { normalizeStructTag } from "@mysten/sui/utils"; | ||
| import { SUI_TYPE } from "./config.js"; | ||
|
|
||
| const COIN_WITH_BALANCE_INTENT = "CoinWithBalance"; | ||
|
|
||
| /** | ||
| * Prevent the SDK's CoinWithBalance resolver from falling back to owned coins | ||
| * when an address balance is insufficient. | ||
| */ | ||
| export function enforceAddressBalanceCoinIntents(transaction: Transaction): void { | ||
| let initialOwnedObjectIds: Set<string> | undefined; | ||
| transaction.addSerializationPlugin(async (transactionData, options, next) => { | ||
| initialOwnedObjectIds ??= new Set( | ||
| transactionData.inputs | ||
| .map((input) => ( | ||
| input.Object?.ImmOrOwnedObject?.objectId ?? input.UnresolvedObject?.objectId | ||
| )) | ||
| .filter((objectId): objectId is string => typeof objectId === "string"), | ||
| ); | ||
| const requiredByType = new Map<string, bigint>(); | ||
| for (const command of transactionData.commands) { | ||
| if (command.$kind !== "$Intent" || command.$Intent.name !== COIN_WITH_BALANCE_INTENT) { | ||
| continue; | ||
| } | ||
| const type = command.$Intent.data?.type; | ||
| const balance = command.$Intent.data?.balance; | ||
| if ( | ||
| typeof type !== "string" | ||
| || (typeof balance !== "bigint" && typeof balance !== "number" && typeof balance !== "string") | ||
| ) { | ||
| continue; | ||
| } | ||
| const coinType = type === "gas" ? SUI_TYPE : normalizeStructTag(type); | ||
| requiredByType.set(coinType, (requiredByType.get(coinType) ?? 0n) + BigInt(balance)); | ||
| } | ||
|
|
||
| if (requiredByType.size > 0) { | ||
| const client = options.client as any; | ||
| if (!client?.core?.getBalance || !transactionData.sender) { | ||
| throw new Error("Address-balance upload requires a sender and Sui client"); | ||
| } | ||
|
|
||
| await Promise.all([...requiredByType.entries()].map(async ([coinType, required]) => { | ||
| const response = await client.core.getBalance({ | ||
| owner: transactionData.sender, | ||
| coinType, | ||
| }); | ||
| const available = BigInt(response?.balance?.addressBalance ?? 0); | ||
| if (available < required) { | ||
| throw new Error( | ||
| `Insufficient ${coinType} address balance: required ${required}, available ${available}`, | ||
| ); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| await next(); | ||
| const newlyResolvedOwnedObjects = transactionData.inputs | ||
| .map((input) => input.Object?.ImmOrOwnedObject?.objectId) | ||
| .filter((objectId): objectId is string => ( | ||
| typeof objectId === "string" && !initialOwnedObjectIds.has(objectId) | ||
| )); | ||
| if (newlyResolvedOwnedObjects.length > 0) { | ||
| throw new Error( | ||
| `Address-balance upload resolved owned coin objects: ${newlyResolvedOwnedObjects.join(", ")}`, | ||
| ); | ||
| } | ||
| if (transactionData.gasData.payment && transactionData.gasData.payment.length > 0) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check can't fire in practice — Suggest moving this specific check into |
||
| throw new Error("Address-balance upload resolved gas from an owned coin"); | ||
| } | ||
| }); | ||
| } | ||
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.
This test's title says "fail instead of falling back to owned coins", but it only calls
transaction.prepareForSerialization(...)(lines 268-274), never.build(...). Gas-coin resolution only happens inside#prepareBuild, whichprepareForSerializationnever reaches — so this test can only exercise the coin-intent-resolved-to-owned-object case (which does correctly reject today), not the gas-payment fallback case the PR summary claims is covered.Suggest adding a case that mocks gas resolution to return an owned coin and asserts
transaction.build({ client })rejects the same way. With the current code, that new test would fail — that's the actual bug this PR needs to fix.