Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

refactor(experimental): add function to check if a transaction is fully signed #1783

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

mcintyre94
Copy link
Contributor

  • asserts that there is a signature for each expected signer
  • only verifies the presence of signatures, does not verify the signatures are valid

Part of #1773, but won't close yet in case we want to add a separate function to validate signatures too

Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Copy link
Contributor

@buffalojoec buffalojoec left a 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.

@mcintyre94 mcintyre94 force-pushed the add-assert-fully-signed-tx branch from 193b243 to e61e916 Compare October 25, 2023 14:44
@mcintyre94
Copy link
Contributor Author

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;

Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sign it

Copy link
Contributor

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okey-panter-tost-e.gif

requiredSigners.forEach(address => {
if (!transaction.signatures[address]) {
// TODO coded error
throw new Error(`Transaction is missing signature for address ${address}`);
Copy link
Contributor

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?

Suggested change
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
@mcintyre94 mcintyre94 force-pushed the add-assert-fully-signed-tx branch from e61e916 to 3d321d0 Compare October 26, 2023 09:28
@mcintyre94 mcintyre94 merged commit 3baf697 into master Oct 26, 2023
6 checks passed
@mcintyre94 mcintyre94 deleted the add-assert-fully-signed-tx branch October 26, 2023 15:10
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.87.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants