Verifier main loop. #1135
Verifier main loop. #1135
Conversation
| // Next returns a channel that will yield the next piece of work to be processed. | ||
| Next(ctx context.Context) <-chan Work | ||
|
|
||
| // Watch returns a channel that will yield all work to be processed. | ||
| Watch(ctx context.Context) <-chan Work |
There was a problem hiding this comment.
We should probably only have one of these, I'm not sure which is better.
winder
force-pushed
the
will/verifier-process
branch
2 times, most recently
from
e159165 to
d92211e
Compare
winder
force-pushed
the
will/verifier-process
branch
from
d92211e to
5ecde9e
Compare
|
| // Reader is an interface for reading the blockchain for new work. | ||
| type Reader interface { | ||
| // Next returns a channel that will yield the next piece of work to be processed. | ||
| Next(ctx context.Context) <-chan Work | ||
|
|
||
| // Watch returns a channel that will yield all work to be processed. | ||
| Watch(ctx context.Context) <-chan Work | ||
| } |
There was a problem hiding this comment.
This needed with the SourceReader interface already defined?
// SourceReader is an interface that provides a way to read messages from the source chain onramp.
// This is expected to be implemented per chain family.
type SourceReader interface {
Service
// Messages returns a channel of messages that are read from the source chain onramp.
Messages() <-chan Message
// GetMessages returns a slice of messages from the source chain onramp starting
// from the given sequence number.
GetMessages(ctx context.Context, query MessageQueryArgs) ([]Message, error)
}There was a problem hiding this comment.
I'd leave out GetMessages and especially query. If we need to rollback the reader for some reason it should be as simple as possible, boil it down to which round to start on and simply stream everything again from that point.
| // Transformer is an interface that defines how to transform a Work item into | ||
| // the generic modsec.Message format and encoded into a destination specific | ||
| // payload. This interface encapsulates most of the chain agnostic services | ||
| // required by the 1.6 implementation. | ||
| type Transformer interface { | ||
| Transform(work Work) HandlerPayload | ||
| } |
There was a problem hiding this comment.
Encapsulated in the interface above - could make things more generic if we think its useful though.
There was a problem hiding this comment.
I think its important to split reading and re-encoding. This layer will have chain family specific re-encoding details. So may as well split it up from the blockchain reading so that each has a single responsibility.
| // Writer is an interface that defines how to write attestations and messages to a storage layer. | ||
| // The writer is responsible for deciding how to store the messages and attestations, including | ||
| // how to avoid duplicates, and how to structure the storage for efficient scanning. | ||
| // | ||
| // TODO: standard io.Writer interface could be used to simplify testing. | ||
| // Modex extensions would look like "modsec.NewS3Writer(path)" | ||
| type Writer interface { | ||
| // WriteMessage stores the message that is being attested. | ||
| // The writer implementation is responsible for deciding how to store the message. | ||
| // It should consider things like how it will be retrieved later, and how to avoid duplicates. | ||
| // For example, it may decide to store messages in a hierarchy based on the time, block number, sequence number, etc. | ||
| WriteMessage(ctx context.Context, msg HandlerPayload) error | ||
|
|
||
| // WriteAttestation stores the attestation for a message. | ||
| // The writer implementation is responsible for deciding how to store the attestation. | ||
| // It should consider things like how it will be retrieved later, and how to avoid duplicates. | ||
| WriteAttestation(ctx context.Context, msg Attestation) error | ||
| } |
There was a problem hiding this comment.
What I landed on:
// AttestationWriter defines the interface for storing attestations.
type AttestationWriter interface {
Store(ctx context.Context, att Attestation) error
}The Attestation is the object that can be used by the implementation to come up with the key path (if backend is S3) and contains the full CCIP message as well as the message proof, so I don't think we need two separate methods.
There was a problem hiding this comment.
I think it could be useful to split up the Message, Attestation(s) and maybe even an optional AttestationData for verifiers that produce something like a CCTP attestation.
OffRamp.Execute should accept these objects as the arguments.
| // TODO: should verifier be a full service with start/stop/cancel, a cache, maybe a db connection, etc? | ||
| type Handler func(ctx context.Context, payload HandlerPayload, result chan<- Attestation) |
There was a problem hiding this comment.
I have the verifier logic encapsulated in an interface:
// Verifier is responsible for performing the specific verification logic for a message.
type Verifier interface {
// Verify tries to verify the given message and returns nil if the message is valid.
// For example, the commit verifier can check that the message is sufficiently deeply buried in the blockchain,
// or is finalized/safe.
Verify(ctx context.Context, message CCIPMessage) error
}Method on it like that is synchronous, though we could have an async version or make it fully async with the channel input.
There was a problem hiding this comment.
I think this assumes that Verify deals with creating the attestation, encoding a dest specific message, signing and writing.
IMO this is way too complex for a component we hope for 3rd parties to implement.
| case <-ctx.Done(): | ||
| fmt.Println("Verifier shutting down handlers...") | ||
| return | ||
| case payload := <-writePayloadCh: |
There was a problem hiding this comment.
Seems kinda overkill to make message encoding an async thing
| } | ||
| case attestation := <-writeAttestationCh: | ||
| for _, w := range s.writer { | ||
| if err := w.WriteAttestation(ctx, attestation); err != nil { |
There was a problem hiding this comment.
Message encoding is required for attestation; if you'd want to make it a full pipeline you'd have to encode first then attest second
| // Attestation for a message. | ||
| type Attestation struct { | ||
| VerifierInfo VerifierInfo | ||
| Message modsectypes.Message |
There was a problem hiding this comment.
Do we need a message in the attestation itself? Shouldn't the msg hash be enough here?
3 participants
Sketch in a main loop for the verification logic.