Multi sig witness#1028
Merged
Merged
Conversation
AlCutter
force-pushed
the
multi_sig_witness
branch
2 times, most recently
from
July 1, 2026 17:43
fc525c6 to
b11b371
Compare
roger2hk
approved these changes
Jul 2, 2026
| seen := make(map[verifierKey]bool) | ||
| var dedup []note.Verifier | ||
| for _, v := range vs { | ||
| k := verifierKey{name: v.Name(), hash: v.KeyHash()} |
Contributor
There was a problem hiding this comment.
Could v be nil here? If so, add a defensive nil check.
if v == nil {
continue
}| @@ -266,10 +289,10 @@ func (pf *sharedConsistencyProofFetcher) ConsistencyProof(ctx context.Context, s | |||
| // This is defaulted to zero on startup and calibrated after the first request, which is expected by the spec: | |||
| // `If a client doesn't have information on the latest cosigned checkpoint, it MAY initially make a request with a old size of zero to obtain it` | |||
Contributor
There was a problem hiding this comment.
Suggested change
| // `If a client doesn't have information on the latest cosigned checkpoint, it MAY initially make a request with a old size of zero to obtain it` | |
| // `If a client doesn't have information on the latest cosigned checkpoint, it MAY initially make a request with an old size of zero to obtain it` |
| } | ||
| if recursed >= maxUpdateRecursion { | ||
| return nil, fmt.Errorf("too many consecutive requests to witness %s", w.verifier.Name()) | ||
| return nil, fmt.Errorf("too many consecutive requests to witness %s", w.verifiers[0].Name()) |
Contributor
There was a problem hiding this comment.
Would putting the first verifier in the error log be confusing later?
Collaborator
Author
There was a problem hiding this comment.
In theory no, because the only use case for this currently is for a witness which returns multiple signatures, so all names should be identical. I've enforced this this is the case when parsing from policy at least.
That said, I could imagine this also supporting some sort of external witness gateway thing in the future, so I've changed to collate & uniq the names into a string here, and used that.
| if n, err := note.Open(signed, note.VerifierList(w.verifier)); err != nil { | ||
| return nil, fmt.Errorf("witness %q at %q replied with invalid signature: %q\nconstructed note: %q\nerror: %v", w.verifier.Name(), w.url, rb, string(signed), err) | ||
| if n, err := note.Open(signed, note.VerifierList(w.verifiers...)); err != nil { | ||
| return nil, fmt.Errorf("witness %q at %q replied with invalid signature: %q\nconstructed note: %q\nerror: %v", w.verifiers[0].Name(), w.url, rb, string(signed), err) |
Contributor
There was a problem hiding this comment.
Would putting the first verifier in the error log be confusing later?
AlCutter
force-pushed
the
multi_sig_witness
branch
from
July 2, 2026 14:12
5cdd19f to
8542623
Compare
AlCutter
force-pushed
the
multi_sig_witness
branch
from
July 2, 2026 14:12
8542623 to
cc90c30
Compare
AlCutter
force-pushed
the
multi_sig_witness
branch
from
July 2, 2026 14:34
cc90c30 to
a1d10d7
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
2 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.
This PR improves the way the Tessera witness client handles policies which require multiple cosignatures from the same URL (e.g. a witness which is returning both
ed25519andMLDSA cosignatures).Previously, such a policy would have caused multiple
add-checkpointrequests to be sent to the witness, one for eachvkeyin the policy, and all but the first of those would be guaranteed to receive a409in response causing more requests to be sent.With the changes in this PR, Tessera should make at most a single call to each witness URL present in the policy (conflicts not withstanding).
I've added a new
WitnessEndpointfunc on the witness policy interface so as to avoid breaking semver by changing the func signature. In practice, I doubt anyone uses the oldEndpointsfunc directly (as opposed to just creating the policy structure and passing it directly toWithWitnesses), but...