Update challenge transaction helpers sep 10 #497
Merged
Conversation
This file contains 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
…i-signature logic
…the readChallengeTx method
… a few unit tests
…' and 'Utils.verifyTxChallengeTxThreshold
… replaced most of the 'verifyChallengeTx' implementation with a call to 'readChallengeTx'
…transaction-helpers-sep-10 * upstream/master: Fix broken link to Stellar logo+wordmark (stellar#496) Junk the _link omition for AccountResponse class (stellar#495) v4.0.0 - Horizon 1.0 support (stellar#488) Add is_authorized to BalanceLineAsset. (stellar#491) Update yarn.lock. (stellar#490) Typescript fixes (stellar#489)
abuiles
suggested changes
Feb 13, 2020
…TxMultiSignedBy' function
…hings clearer in case it ever happens.
leighmcculloch
requested changes
Feb 14, 2020
There was a problem hiding this comment.
Thanks making this change @marcelosalloum! There are a couple small things I think we need to tweak, and I have a couple questions that I suspect might be me misunderstanding something about Typescript. They are noted in line below. Otherwise this looks great. Great job on the tests too!
…hold' function) from a 'Map<string, number>' to a 'ServerApi.AccountRecordSigners[]'
abuiles
approved these changes
Feb 17, 2020
There was a problem hiding this comment.
LGTM! @leighmcculloch can you have a final view before we merged it.
…ts that were lacking them
…r starts with G... and the other verifies if it is a valid 'Ed25519'.
…hen calling `Keypair.fromPublicKey`
…ansaction-helpers-sep-10 * origin/master: Extend offer call builder to return a single offer. (stellar#499)
marcelosalloum
deleted the
update-challenge-transaction-helpers-sep-10
branch
marcelosalloum
restored the
update-challenge-transaction-helpers-sep-10
branch
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR updates the challenge transaction verifiers to accept multiple signatures, according to the issue #483
What
Add new challenge transactions helpers to
utilsto support verifying a challenge transaction that has multiple signatures. Three new functions were added and an existent function was deprecated:readChallengeTx: reads the challenge details to parse the transaction and the client account ID out of the transaction.verifyChallengeTxSigners: verify that the signers of the transaction are a subset of a list of public keys passes as arguments.verifyChallengeTxThreshold: verify that the signers of the transaction are a subset of a list of signers that meet a required threshold.verifyChallengeTxWhy
The SEP-10 change that was merged in stellar/stellar-protocol#489 clarifies how an implementer should verify that signers of the transaction are signers on the account and that accounts may have multiple signers.
An implementer needs to read out of the transaction the client account before verifying the transaction and then its threshold. For that reason we need three steps:
readChallengeTx)verifyChallengeTxSigners)verifyChallengeTxThreshold)The read call also validates the server signature because no challenge transaction would ever be valid to read if it wasn't signed by the server.
For further info, please refer to the SEP-10 protocol.