Wire transaction events into devnet#3633
Conversation
be7562f to
8d22450
Compare
0887a65 to
1e55997
Compare
| COPY --from=base-routing proxyd/ ./proxyd/ | ||
| RUN cd proxyd && make proxyd | ||
|
|
||
| FROM 652969937640.dkr.ecr.us-east-1.amazonaws.com/containers/golang:v1-24-ub22 |
There was a problem hiding this comment.
The runtime stage uses the full Go SDK image (golang:v1-24-ub22) rather than a minimal base. The original Dockerfile.proxyd uses alpine:3.20 for the runtime, which is orders of magnitude smaller. Since the runtime only needs the proxyd binary (and curl for healthchecks), consider using a slim base image to reduce image size and attack surface.
Also worth noting: the healthcheck in docker-compose.ingress.yml runs curl, but this Dockerfile doesn't explicitly install it. It may work because the Ubuntu-based Go image likely ships curl, but this is an implicit dependency — if the base image changes, the healthcheck will silently break.
| base_meteredPriorityFeePerGas = "client" | ||
|
|
||
| [transaction_events] | ||
| enabled = false |
There was a problem hiding this comment.
The TOML config sets enabled = false, but the docker-compose.ingress.yml sets TRANSACTION_EVENTS_ENABLED=true as an environment variable. If proxyd uses env-var overrides for its TOML config, this works as intended (TOML default = off, env override = on). But if proxyd reads the TOML file without env-var override support, transaction events won't be enabled despite the env var.
Could you confirm that proxyd supports environment variable overrides for the [transaction_events] TOML section? If it does, this pattern is fine. If not, this should be enabled = true (or the env var approach is dead code).
| type: http | ||
| inputs: | ||
| - parse_transaction_events | ||
| uri: http://audit-archiver:9100/v1/transaction-events/batch |
There was a problem hiding this comment.
The audit-archiver port is hardcoded to 9100 here. While Vector YAML doesn't support Docker Compose variable interpolation, this creates a silent coupling with AUDIT_RPC_PORT=9100 in devnet-env. If someone changes AUDIT_RPC_PORT, they'd also need to find and update this file — consider adding a comment noting this dependency.
8d22450 to
f36f90d
Compare
f36f90d to
0d01740
Compare
Review SummaryThis PR wires transaction event observability into the devnet infrastructure — adding Vector as a log collector, Postgres for event storage, a smoke test script, and environment/compose configuration for the full pipeline. No Rust source code changes beyond a single CLI flag addition. FindingsThe inline comments from the previous review run cover the main issues:
Other observations (no action needed)
|
✅ base-std fork tests: all 616 passedbase/base is fully in sync with the base-std spec.
|
3772911 to
7323f40
Compare
Summary
Verification
Stack Context
This PR is based on #3628 and contains only devnet/Vector wiring for local verification. It is intentionally reviewed separately from service producer code.
type=routine
risk=low
impact=sev5