feat(node): guardian signed delegate signatures broadcast#4744
Conversation
547444a to
33e112f
Compare
c93a95e to
5ea4844
Compare
|
I'll analyze this and get back to you. |
5ea4844 to
5aa5fae
Compare
There was a problem hiding this comment.
Given that we're doing checks against the current Guardian Set on one level, and the delegated/canonical sets on another level, do we need to worry about the boundary cases like:
- Changing a Guardian Set?
- Changing the Delegated Guardian Set for a chain?
16347df to
966587a
Compare
johnsaigle
left a comment
There was a problem hiding this comment.
I think we should update the test coverage to make sure we're handling the non-VAA-hash-related fields correctly.
Otherwise, the rest of the comments are improvements but IMO they do not need to block the PR.
mdulin2
left a comment
There was a problem hiding this comment.
Looks pretty good to me! The checks on the single signature appear to all be done on the batch case as well.
This reverts commit 628c06e.
9be839d to
937d18f
Compare
johnsaigle
left a comment
There was a problem hiding this comment.
A few small notes for test coverage and logging in case we hit size limits on the p2p layer.
Otherwise looks good and I'm happy to approve once these are resolved.
djb15
left a comment
There was a problem hiding this comment.
Approving changes to gossip.proto

This returns the gossip message time restriction pre #4743 in favor of a canonical guardian only approach. This allows any guardian to collect a quorum of delegated guardian set signatures for a message and broadcast them in a single message.
The goal is that this should maintain the same safeguards and security of the original delegated guardian set messages but provide the same security / DoS prevention as observation requests.
Tested with https://api.wormholescan.io/api/v1/observations/delegate/50/00000000000000000000000062deeafee06c7442a21c93ededc79a0cb5791c83/1750
This required a change in Wormholescan to store the full data from the gossip message, so some earlier messages may not contain all the relevant fields. I expect that if a prior message was needed for some reason, it could either be re-observed or the data could be backfilled.
Reviewers: the most critical code is in p2p.go 👀
Summary