batchVAA guardian changes

[justinschuldt]

No description provided.

[evan-gray]

Part of #1679

[justinschuldt]

Addresses #1857

[justinschuldt]

@bruce-riley, @jynnantonix, @tbjump I'm going to kick this to WIP while I put the BatchVAA generation (this entire codepath) behind a feature flag.

I'm also going to update the channels in this PR to be typed as read or write, in the style of @tbjump #1931, as the conflict is imminent.

[justinschuldt]

@tbjump I decided against implementing the directional Go channel changes, as they require 1.19, and the change would have been messy if I would have implemented the same generics in this PR.

I think the best course of action is to just see whatever PR lands first, then address the merge conflicts.

[jynnantonix]

The general idea seems fine but duplicating the core processor logic so that we have 2 different code paths that are almost, but not quite the same is just asking for trouble. For example, the copied code is missing the re-observation rate-limit on the sender side (#1621) and deduplication on the receiver side (#1462).

My understanding is that @hendrikhofstadt 's processor refactor (#1953) should make it much easier to implement batch handling so I suggest we either wait for that to merge or stack this PR on top of that one (graphite's CLI makes it easy to do this).

[hendrikhofstadt]

The general idea seems fine but duplicating the core processor logic so that we have 2 different code paths that are almost, but not quite the same is just asking for trouble. For example, the copied code is missing the re-observation rate-limit on the sender side (#1621) and deduplication on the receiver side (#1462).

My understanding is that @hendrikhofstadt 's processor refactor (#1953) should make it much easier to implement batch handling so I suggest we either wait for that to merge or stack this PR on top of that one (graphite's CLI makes it easy to do this).

Strongly agree with @jynnantonix here. I think the general idea looks good.
One design level question I have is the pattern of batch requests and transaction queries.
Is there a reason for following this pattern instead of having the watcher immediately scan for batches and individual messages. Most processing on the watcher level should happen on a per-block / tx basis which should provide enough data to immediately identify batches and therefore reduce complexity significantly. Obviously it pushes more logic to the watcher but generally sounds more reliable.

[justinschuldt]

I'm using this for reference as I make a new/similar PR on top of Hendrik's refactor PR.
I'm leaving this PR open for the time being, will ultimately close in favor of the new branch & PR.

[justinschuldt]

Here are some things to consider when looking at this, and an update on where this sits:

implementation details

• Batch VAAs need a different identifier compared to v1 VAAs as messageID is specific to an individual message (unique sequence per message emitted). Batch VAAs are unique to the transaction ID and the nonce. There can be multiple batches from a transaction, messages are grouped into batches by matching nonce. BatchID could be {emitterChain}:{transactionID}:{nonce}, where transaction ID is sufficiently long and left-padded, to hold the full IDs from chains like Solana.
  • reference PR Go SDK structs for BatchID.
• The transaction ID of a batch is not included in the batch's VAA. The transaction ID is included in the gossip messages of batches. Keeping the transaction ID along side the batch VAA is needed, because how else would you index/reference a batch without it, and also for things like the spy, so users/relayers can query for a specific batch VAA.
• The solana transaction hash produced by the solana watcher is not the transaction identifier from the chain. The base58 account ID from Solana is concatenated to fit inside an eth hash here. The solana watcher will need to be updated to produce a transaction ID that matches the chain's, rather than account ID, or users will have to jump through hoops to translate what they get from the chain to the format produced by the guardian. I made an issue to track this. Fixing is not needed for the EVM implementation, but should be considered when designing the BatchVAA IDs.

Completed:

• BatchVAA gossip message protos. PR
• BatchVAA Go structs, with marshaling/unmarshaling, and signature logic. PR, update to separate BatchID from Batch VAA sdk/vaa/structs.go
• Spy integration - watching gossip for batch messages and client subscriptions with filtering. Disabled currently, enable by adding this. PR

Needs to be added after Hendrik's guardian refactor #1953:

• Get the source-of-truth about the messages within a transaction. Fetch the messages within a transaction from the chain, so we know when all the individual messages have been signed. This could be very similar to the observation request functionality that currently exists in the guardian.
  • Exists in the reference PR as a TransactionQuery which is sent to watchers through a channel batchReqC (similar to obsvReqC). The resulting data is stored as TransactionData. Reference implementation
• Create a map of transaction ID to message ID, and another vice versa, so v1 VAAs can be associated with a tx. This is needed because v1 VAAs do not contain info about the transaction. Currently when a message is observed by the guardian a MessagePublication holds it's info, and then a subset of the MessagePublication becomes the VAA. VAAs that come from gossip have no way of being tied back to a transaction.
  • These maps are stored in processor state in the reference PR.
• Determine when all of the messages within a TX have reached quorum. for example, when a VAA reaches quorum, check to see if it is part of a batch, and if so, if the batch is now complete (all messages within have a VAA).
  • exists in the reference PR as node/pkg/processor/batch.go
• Gossip channels and message handlers for BatchVAA gossip messages.
  • exists in the reference PR in the old style processor (node/pkg/processor/processor.go), will need to be updated to work with the new style.
• Once all the messages in the batch have reached quorum, make the BatchVAA, sign it, and broadcast it to gossip. At this point the logic for BatchVAAs is the same as v1 VAAs: collect signatures from peers, check for quorum, broadcast the signed batch VAA when quorum is reached. I imagine a reactor.NewManager would be used for this.
• Create a DB prefix for BatchVAAs, indexed by BatchID, and DB handler functions for storing and retriveing them.
  • This can probably be taken wholesale from the reference PR node/pkg/db/db.go, should not need modification.
• publicrpc endpoint for batch VAAs. Same idea as the endpoint for v1 VAAs, just for batches. Takes a BatchID and returns a BatchVAA.
  • reference PR node/pkg/publicrpc/publicrpcserver.go, should not need modification.

I'm closing this PR now as it makes more sense to make a fresh PR after #1953 lands, rather than rebasing and updating this.