Allow signing a transaction that does not have TransactionWithLifetime #1058
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: 1bcc4c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BundleMonUnchanged files (133)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
mcintyre94
force-pushed
the
sign-wo-lifetime
branch
from
0852290 to
1bcc4c0
Compare
Contributor
|
Documentation Preview: https://kit-docs-977pmwlrg-anza-tech.vercel.app |
lorisleiva
approved these changes
Nov 28, 2025
Member
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Since I got a bit confused by the potential consequences of this PR and Callum reassured me in DMs, I just want to flag here that this does not create a loss of type safety in the following consumer workflow: "TransactionMessage → compile/sign → Transaction → send".
This is because the TransactionMessage contains lifetime information and the functions that will return a Transaction object from it will ensure a lifetime has been specified.
However, the Transaction type itself only contains the compiled message as its source of truth. The TransactionWithLifetime type is an additional metadata flag we added to the object to easily identify which lifetime is set within the compiled message. Removing this requirement to the signing helper is only removing the need to have this redundant lifetime identifier when signing.
Contributor
|
cc/ @Jac0xb. |
This was referenced Dec 1, 2025
Merged
Closed
Closed
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
I think we went a bit far here. It's important that
signTransactionandpartiallySignTransactionreturn the sameTTransaction extends Transactionthat is passed to them, so that they eg. preserve the lifetime. But I think it is ok to sign aTransactionthat does not have a lifetime field attached to it. One place this is useful is a server that receives serialized transactions to sign, it should be able to sign the result oftransactionDecoder.decode(transactionBytes)without concerning itself with that transaction's lifetime.Summary of Changes
Remove
TransactionWithLifetimefrom the required input type ofsignTransactionandpartiallySignTransaction.