Skip to content

fix variable-length array destructuring bug in spend-permissions complete integration example#1525

Open
weirdDevelop wants to merge 1 commit into
base:masterfrom
weirdDevelop:master
Open

fix variable-length array destructuring bug in spend-permissions complete integration example#1525
weirdDevelop wants to merge 1 commit into
base:masterfrom
weirdDevelop:master

Conversation

@weirdDevelop
Copy link
Copy Markdown

Summary

The Complete Integration Example in docs/base-account/improve-ux/spend-permissions.mdx
contains a workflow bug that causes silent runtime failures on repeat executions of a
spend permission.

Problem

Bug 1 — Fixed destructuring of a variable-length array

The guide currently does:

const [approveCall, spendCall] = await prepareSpendCallData({ permission, amount });
calls: [approveCall, spendCall]

According to the prepareSpendCallData reference page, the function returns either 1 or 2
calls
depending on whether the permission is already registered onchain:

  • First use: returns [approveWithSignatureCall, spendCall] — 2 elements
  • Subsequent uses: returns [spendCall] — 1 element

When only 1 element is returned, spendCall becomes undefined. The guide then passes
[approveCall, undefined] directly into wallet_sendCalls, which fails at runtime with no
clear error. The first execution succeeds, every subsequent one silently breaks — making
this extremely hard to diagnose.

Bug 2 — Missing unit context on remainingSpend check

The guide checks:

const { isActive, remainingSpend } = await getPermissionStatus(permission);
if (!isActive || remainingSpend < amount) { ... }

remainingSpend is returned in the token's smallest unit (wei for ETH, 6-decimal units
for USDC). There is no unit guidance anywhere on the page, so a developer comparing
remainingSpend to a human-readable amount value will get incorrect logic for any
ERC-20 token that isn't 18 decimals.

Fix

Bug 1 — Replace fixed destructuring with the full array, matching what the
prepareSpendCallData reference page itself demonstrates:

// Before
const [approveCall, spendCall] = await prepareSpendCallData({ permission, amount });
calls: [approveCall, spendCall]

// After  
const spendCalls = await prepareSpendCallData({ permission, amount });
calls: spendCalls

Bug 2 — Add a one-line inline comment after the remainingSpend comparison clarifying
units:

// remainingSpend and amount are in the token's smallest unit
// (e.g. for USDC with 6 decimals, 1_000_000n = $1.00)

Verification

  • prepareSpendCallData return type confirmed against:
    /base-account/reference/spend-permission-utilities/prepareSpendCallData
  • getPermissionStatus return shape confirmed against:
    /base-account/reference/spend-permission-utilities/getPermissionStatus
  • The reference page's own example uses spendCalls as a whole array — never
    destructured. The guide contradicts its own reference.

Impact

This is a timing-dependent failure. The first execution always succeeds (2-element array,
destructuring works). Every subsequent execution silently fails (1-element array,
spendCall = undefined passed to wallet_sendCalls). Developers who test once and ship
will hit this in production.

Files Changed

  • docs/base-account/improve-ux/spend-permissions.mdx

…gration example prepareSpendCallData returns 1 call when permission is already registered and 2 calls on first use. The guide destructures it as a fixed 2-element array, causing spendCall=undefined on repeat executions and silently passing undefined into wallet_sendCalls. Replaces fixed destructure with the full array, matching the reference page's own example. Also adds unit clarification for remainingSpend.
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/2
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

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