Skip to content

fix(spend-permission): use wallet-signed data for returned permission#332

Open
faisalnugroho wants to merge 1 commit into
base:masterfrom
faisalnugroho:fix/spend-permission-inconsistency
Open

fix(spend-permission): use wallet-signed data for returned permission#332
faisalnugroho wants to merge 1 commit into
base:masterfrom
faisalnugroho:fix/spend-permission-inconsistency

Conversation

@faisalnugroho

Copy link
Copy Markdown

Fixes #324

Problem

requestSpendPermission uses mutableData: { fields: ['message.account'] } to allow the wallet to substitute the account address (e.g., replacing an EOA with a smart-wallet). However, the returned SpendPermission object uses the original pre-substitution typedData.message for the permission field, while correctly using the post-substitution signResult.signedData.message for permissionHash.

This produces an internally inconsistent object:

result.permissionHash  → hash of { account: 0xSmartWallet, ... }  // correct
result.permission.account → 0xCallerEOA                           // WRONG

Fix

Track the actual signed message and use it consistently:

  • Introduced signedMessage variable initialized to typedData.message
  • In the wallet_sign path, update signedMessage = signResult.signedData.message
  • Use signedMessage in the returned SpendPermission object

This ensures permission.account always matches the account in permissionHash, regardless of whether the wallet performed a substitution.

Testing

The existing test mocks signedData as the same object as typedData, so no substitution occurs. A test where signedData.message.account differs from the request account would expose the inconsistency. This fix makes such a test pass correctly.

When capabilities are provided, the wallet may substitute message.account
via mutableData (e.g., replacing an EOA with a smart-wallet address).
The returned SpendPermission object was using the original pre-substitution
typedData.message for the permission field, while correctly using the
post-substitution data for permissionHash. This created an internally
inconsistent object where permission.account != the address in the hash.

Track the actual signed message and use it consistently in the returned
SpendPermission object.

Fixes base#324
@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
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

@faisalnugroho

Copy link
Copy Markdown
Author

Hi! I'd love to get this PR reviewed when you have a moment. This PR addresses use wallet-signed data for returned permission. Happy to make any adjustments based on your feedback. Let me know if there's anything I can do to help move this forward.

@faisalnugroho

Copy link
Copy Markdown
Author

Hi! I'd love to get this PR reviewed when you have a moment. This PR addresses use wallet-signed data for returned permission. Happy to make any adjustments based on your feedback. Thank you!

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.

spend-permission: requestSpendPermission returns inconsistent SpendPermission when wallet substitutes account via mutableData

2 participants