Update useWalletAccountTransactionSigner to return a LifetimeConstraint for the updated transaction #919
+223
−8
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
🦋 Changeset detectedLatest commit: 9895e9d The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Member
Author
BundleMonFiles updated (3)
Unchanged files (127)
Total files change +916B +0.25% Final result: ✅ View report in BundleMon website ➡️ |
Contributor
|
Documentation Preview: https://kit-docs-ralhklsip-anza-tech.vercel.app |
mcintyre94
changed the base branch from
transaction-lifetime-construct
to
graphite-base/919
mcintyre94
force-pushed
the
graphite-base/919
branch
from
abc3ad2 to
525e9ae
Compare
mcintyre94
changed the base branch from
graphite-base/919
to
transaction-lifetime-construct
mcintyre94
force-pushed
the
wallet-hook-lifetime
branch
from
0c17c5d to
28ae2b1
Compare
This was referenced Oct 1, 2025
steveluscher
requested changes
Oct 1, 2025
packages/react/src/__tests__/useWalletAccountTransactionSigner-test.ts
Outdated
Show resolved
Hide resolved
| } catch (e) { | ||
| if (isSolanaError(e, SOLANA_ERROR__TRANSACTION__NONCE_ACCOUNT_CANNOT_BE_IN_LOOKUP_TABLE)) { | ||
| throw new SolanaError(SOLANA_ERROR__SIGNER__NONCE_ACCOUNT_CANNOT_BE_IN_LOOKUP_TABLE, { | ||
| signedTransaction, |
Collaborator
There was a problem hiding this comment.
So, I presume that you attach the signedTransaction to the error context in the case that the caller wants to say ‘I don't care about the lifetime, just give me the damn signature.’
Tricky. We can't stuff signatures into the error context for fear of logging, but I can't think of another way of getting them out of this callback.
How about we just punt on this case, throw your error, and offer no remediation. To say it back to you, the conditions that would have to stack up for this error to ever get hit are:
- A proposer would need to be using nonces, AND
- That nonce account would have to be found only in an address lookup table, AND
- Someone would have to be using a Wallet Standard wallet to sign that transaction, AND
- The wallet would have had to have modified the transaction before signing
Other than being a lot of ducks to line up, all in a row, for things to go wrong, I think it's actually pretty unlikely. The primary use case for nonce transactions is to make it possible for multiple signers to participate in a transactions in cases where it will take longer than the expiry of a blockhash to organize all of them. If you have a wallet modifying the transaction message in such a case, your signature will mismatch with the others unless their wallets also made the exact same modification. Unlikely.
Member
Author
There was a problem hiding this comment.
Yep it was just to provide a recovery mechanism when you get that SOLANA_ERROR__TRANSACTION__NONCE_ACCOUNT_CANNOT_BE_IN_LOOKUP_TABLE edge case. I figured that you'd still get the signed transaction in your app, you just wouldn't have a lifetime - but you might be able to recover depending what your app needs.
I agree it's unlikely and that logging the transaction is too dangerous. I'll just drop the custom error here, not worth the risk and - as you say - very unlikely to happen in practice.
Member
Author
There was a problem hiding this comment.
Okay I've removed the specific error and error handling here. It'll just throw SOLANA_ERROR__TRANSACTION__NONCE_ACCOUNT_CANNOT_BE_IN_LOOKUP_TABLE in this annoying edge case now.
packages/errors/src/context.ts
Outdated
| address: string; | ||
| }; | ||
| [SOLANA_ERROR__SIGNER__NONCE_ACCOUNT_CANNOT_BE_IN_LOOKUP_TABLE]: { | ||
| signedTransaction: Uint8Array; |
Collaborator
There was a problem hiding this comment.
We have to keep in mind that in production, the error context object gets serialized and logged. We must keep sensitive information out of error context, and this probably qualifies as sensitive enough to exclude.
mcintyre94
force-pushed
the
wallet-hook-lifetime
branch
from
28ae2b1 to
8a6cc93
Compare
mcintyre94
force-pushed
the
transaction-lifetime-construct
branch
from
525e9ae to
11136da
Compare
mcintyre94
force-pushed
the
transaction-lifetime-construct
branch
from
11136da to
7171940
Compare
mcintyre94
force-pushed
the
wallet-hook-lifetime
branch
from
8a6cc93 to
3536388
Compare
Member
Author
Merge activity
|
mcintyre94
changed the base branch from
transaction-lifetime-construct
to
graphite-base/919
…nt for the updated transaction
mcintyre94
force-pushed
the
wallet-hook-lifetime
branch
from
3536388 to
9895e9d
Compare
mcintyre94
added a commit
that referenced
this pull request
Oct 3, 2025
…#927) #### Problem The `TransactionModifyingSigner` was previously typed to return the exact same type `T` as its input transactions. This is incorrect, as we always intended for signers to be able to eg change the lifetime of a transaction. In practice one example of `TransactionModifyingSigner` returns signed transactions from wallet-standard wallets, which can make arbitrary changes to the transaction before signing it. This also meant that such a signer could not return a `TransactionWithLifetime`, ie a `lifetimeConstraint` field. Casts in signer functions would treat the signer as having returned the expected type, but there was no type safety. In practice, this meant `useWalletAccountTransactionSigner` and third-party `TransactionModifyingSigner` did not return a `lifetimeConstraint`, but appeared to do so to Typescript. This led to a runtime error when attempting to confirm transactions (#891) #### Summary of Changes This PR changes the type of modifying signers to: **input**: (Transaction | (Transaction & TransactionWithLifetime))[] **output**: (Transaction & TransactionWithLifetime & TransactionWithinSizeLimit)[] Note that an upstream PR (#919) already made `useWalletAccountTransactionSigner` satisfy the `TransactionWithLifetime` part of this interface. It also changes the return type of `signModifyingAndPartialTransactionSigners` to `Transaction & TransactionWithLifetime & TransactionWithinSizeLimit`, and removes the cast to the expected return type that limited type safety in this function. Fixes #891
This was referenced Oct 3, 2025
Merged
Contributor
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Problem
Currently this signer only returns what is required for the signer interface, a
Transaction. However, per #891 this interface needs to be changed to return aTransaction & TransactionWithLifetime, ie augmented with alifetimeConstraintfield.Summary of Changes
The signer now returns a lifetime, by decoding the returned
messageBytesand comparing the returnedlifetimeTokenwith the existing lifetime of the input transaction.lifetimeTokenof the signed transaction matches the existing lifetime (either blockhash or nonce field), then we return the existing lifetime. Ie if the wallet modifies the transaction but not its lifetime.This pre-emptively makes
useWalletAccountTransactionSignercomply with a stricter interface forTransactionModifyingSignerthat will require returningTransaction & TransactionWithLifetime.