fix(token): preserve auditor signatures in Request.SetSignatures - #1905
Merged
adecaro merged 1 commit intoJul 15, 2026
Merged
Conversation
adecaro
self-requested a review
July 14, 2026 03:56
adecaro
force-pushed
the
fix/set-signatures-preserve-auditor
branch
from
July 14, 2026 03:57
fc6eaa7 to
e040806
Compare
adecaro
reviewed
Jul 14, 2026
adecaro
reviewed
Jul 14, 2026
Contributor
There was a problem hiding this comment.
@YanYuanFE , left a comment. Please, have a look.
YanYuanFE
force-pushed
the
fix/set-signatures-preserve-auditor
branch
from
July 14, 2026 08:20
e040806 to
69e5bb4
Compare
adecaro
reviewed
Jul 14, 2026
Contributor
|
@YanYuanFE , thanks for the prompt update. Another small thing and we are good to merge 🙏 |
YanYuanFE
force-pushed
the
fix/set-signatures-preserve-auditor
branch
from
July 14, 2026 13:40
8977c2a to
75fb814
Compare
adecaro
self-requested a review
July 14, 2026 13:41
adecaro
force-pushed
the
fix/set-signatures-preserve-auditor
branch
2 times, most recently
from
July 14, 2026 18:15
bae6de7 to
e6ce76e
Compare
Since LFDT-Panurus#1717 action and auditor signatures share the unified Actions.Signatures slice, but SetSignatures still assigned the whole field, silently dropping auditor signatures attached beforehand via AddAuditorSignature. Endorsement then fails with 'auditor signatures missing' when auditing runs before endorsement collection. Append the action signatures to the ones already attached, restoring the pre-LFDT-Panurus#1717 order-independence of the two calls, and rename SetSignatures to AppendSignatures so the name states that contract. Fixes LFDT-Panurus#1897 Signed-off-by: YanYuan <yan950414@gmail.com>
adecaro
force-pushed
the
fix/set-signatures-preserve-auditor
branch
from
July 15, 2026 05:35
c4bc1d2 to
88b6d35
Compare
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
3 participants
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.
Motivation
Since #1717 action signatures and auditor signatures share the unified
Actions.Signaturesslice, butRequest.SetSignatureskept its whole-field assignment. Any auditor signature attached beforehand viaAddAuditorSignatureis silently discarded, and endorsement later fails incommon.AuditingSignaturesValidatewithauditor signatures missing.We hit this in our test environments with a flow that requests auditing before collecting endorsements (passing
WithSkipAuditing()toCollectEndorsementsView). The SDK's default flow is unaffected because it happens to callSetSignaturesbeforerequestAudit. See #1897 for the full analysis; maintainers confirmed there that this is an unintended side effect of #1717.Changes
SetSignaturesnow preserves existing.Auditorentries when assigning action signatures, restoring the pre-Aggregated Protobuf Best Practices Analysis & Improvement Plan #1622 #1717 order-independence ofAddAuditorSignatureandSetSignatures. The resulting slice order (action entries first, auditor entries after) matches what the default flow produces.TestRequest_SetSignatures_PreservesAuditorSignatures: an auditor signature attached beforeSetSignaturessurvives the call, and a secondSetSignaturescall does not duplicate it.Fixes #1897