fix(fxconfig/transaction): prevent duplicate endorsements per MSP#250
Open
Prachi194agrawal wants to merge 1 commit into
Open
fix(fxconfig/transaction): prevent duplicate endorsements per MSP#250Prachi194agrawal wants to merge 1 commit into
Prachi194agrawal wants to merge 1 commit into
Conversation
Signed-off-by: Prachi Agrawal <prachi194agrawal@gmail.com>
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
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.
Type of change
Description
Endorseintools/fxconfig/internal/transaction/endorse.gopreviously only checked whethertx.Endorsementswasnilbefore appending endorsements. As a result, callingEndorsea second time with the same signer on a transaction that already carried endorsements would silently append a secondEndorsementWithIdentitywith the same MSP ID for each namespace, contrary to the intent expressed in the function comment and to the deduplication behavior ofmergeEndorsements. [web:21]This PR updates
Endorseto prevent duplicate endorsements from the same MSP per namespace:EndorsementWithIdentity, the function now scans existing entries for the current namespace.Identity.MspIdis found,Endorsereturns a descriptive error of the form:duplicate endorsement: signer %q already endorsed namespace %d.This change makes
Endorseconsistent with its own comment (“check that tx does not yet carry any endorsements”) and withmergeEndorsements, which already deduplicates endorsements by MSP ID. It also prevents transactions from leavingEndorsein a state where the same MSP ID appears more than once for a given namespace, which could cause confusing downstream validation behavior. [web:21]Additional details (Optional)
A new regression test,
TestEndorse_DuplicateSignerReturnsError, has been added totools/fxconfig/internal/transaction/endorse_test.go. This test:Endorseonce with a mock signer (e.g.Org1MSP) and asserts that one endorsement is present.Endorsea second time with the same signer and asserts that:nil.duplicate endorsement. [web:21]This ensures the duplicate-protection behavior is exercised and prevents regressions where duplicate endorsements might be silently appended again.
Related issues