-
Notifications
You must be signed in to change notification settings - Fork 772
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
refactor(experimental): add function to check if a transaction is fully signed #1783
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
10bd69c
to
193b243
Compare
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.
Awesome job, just one request: can we get a type test for this? Just something that demonstrates the transaction can now be used as an IFullySignedTransaction
.
193b243
to
e61e916
Compare
Yep good shout! Added this: // assertTransactionIsFullySigned
const transaction = {} as Parameters<typeof assertTransactionIsFullySigned>[0];
// @ts-expect-error Should not be fully signed
transaction satisfies IFullySignedTransaction;
assertTransactionIsFullySigned(transaction);
transaction satisfies IFullySignedTransaction; |
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.
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.
requiredSigners.forEach(address => { | ||
if (!transaction.signatures[address]) { | ||
// TODO coded error | ||
throw new Error(`Transaction is missing signature for address ${address}`); |
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.
Can I be annoying and ask for this in nice copy-pastable markdown format?
throw new Error(`Transaction is missing signature for address ${address}`); | |
throw new Error(`Transaction is missing signature for address \`${address}\``); |
…ly signed - asserts that there is a signature for each expected signer - only verifies the presence of signatures, does not verify the signatures are valid
e61e916
to
3d321d0
Compare
🎉 This PR is included in version 1.87.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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. |
Part of #1773, but won't close yet in case we want to add a separate function to validate signatures too