Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(PROTO-329): Implement Log Fetcher #11

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Conversation

jackchuma
Copy link
Collaborator

@jackchuma jackchuma commented Oct 26, 2024

Description

The Log Fetcher serves as the first component in our fulfiller architecture. Its main purpose is to monitor events from RIP7755Outbox contracts on supported chains. When it ingests an event representing a cross-chain call request, it first parses the log into an ingestible format. It then validates the request by ensuring all routing information matches pre-defined chain configs for the source / destination chains. It then validates that the reward asset / amount represents a reward that would guarantee profit if this request were accepted by the system. If the request passes validation, the log fetcher passes it along to an SQS queue for further processing.

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Oct 26, 2024

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

go-filler PR review suggestions
ArbitrumSepolia: getEnvStr("ARBITRUM_SEPOLIA_RPC"),
BaseSepolia: getEnvStr("BASE_SEPOLIA_RPC"),
OptimismSepolia: getEnvStr("OPTIMISM_SEPOLIA_RPC"),
Sepolia: getEnvStr("SEPOLIA_RPC"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would recommend replacing this with a more robust flags library. There's the built in one: https://pkg.go.dev/flag. OP uses github.com/urfave/cli/v2 heavily, which also supports envvars (e.g. https://github.com/ethereum-optimism/optimism/blob/develop/op-batcher/flags/flags.go).

Copy link

@mdehoog mdehoog Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also might consider loading a toml file or something for the RPCs, given this is per-chain config. Perhaps can merge with the internal/chains/config.go file.

}

func (l *listener) Start(ctx context.Context) error {
sub, err := l.outbox.WatchCrossChainCallRequested(&bind.WatchOpts{}, l.logs, [][32]byte{})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and the previous implementation using SubscribeFilterLogs) only supports websocket RPCs, which tend to be more rare than HTTP RPCs. Would recommend generalizing this and supporting both, with a polling interval for the HTTP one.

}, nil
}

func (l *listener) Start(ctx context.Context) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to somehow track which block we've processed, so we can always start at the most recent unprocessed block. Perhaps writing this metadata to the datastore would be best.

@cb-heimdall
Copy link
Collaborator

Review Error for dav3yblaz3 @ 2024-11-19 02:48:36 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for dav3yblaz3 @ 2024-11-19 02:57:13 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for dav3yblaz3 @ 2024-11-19 02:59:32 UTC
User must have write permissions to review

@cb-heimdall
Copy link
Collaborator

Review Error for dav3yblaz3 @ 2024-11-19 02:59:53 UTC
User must have write permissions to review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants