Skip to content
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

adds multisig/aggregateSigningShares RPC #4677

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

hughy
Copy link
Contributor

@hughy hughy commented Feb 5, 2024

Summary

takes a publicKeyPackage, an unsignedTransaction, a signingPackage for that transaction, and a list of signatureShares for that signingPackage

uses UnsignedTransaction.signFrost to aggregate the signatureShares, sign the transaction, and produce a signed transaction

returns a signed transaction serialized as hex

Testing Plan

RPC integration test will be added in a followup PR

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and
what additional work is required, if any.

[ ] Yes

takes a publicKeyPackage, an unsignedTransaction, a signingPackage for that
transaction, and a list of signatureShares for that signingPackage

uses UnsignedTransaction.signFrost to aggregate the signatureShares, sign the
transaction, and produce a signed transaction

returns a signed transaction serialized as hex
@hughy hughy requested a review from a team as a code owner February 5, 2024 21:50
@@ -821,6 +823,15 @@ export abstract class RpcClient {
},
}
multisig = {
aggregateSignatureShares: (
Copy link
Contributor

Choose a reason for hiding this comment

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

We should either rename this to match signFrost or rename signFrost to match this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merging as is for now, we'll settle on naming in a followup

@jowparks jowparks changed the title adds multisig/aggregateSignatureShares RPC adds multisig/aggregateSigningShares RPC Feb 5, 2024
Copy link
Contributor

@jowparks jowparks left a comment

Choose a reason for hiding this comment

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

Looks good, we just need to rename this (or signFrost) and in a subsequent PR write integration test like you suggested offline.

@hughy hughy merged commit a32b217 into staging Feb 5, 2024
4 checks passed
@hughy hughy deleted the feat/hughy/ifl-2030/rpc-aggregate branch February 5, 2024 23:21
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.

2 participants