Skip to content

feat(load-tests): generalize batch client for validity submissions - #4398

Open
refcell wants to merge 1 commit into
mainfrom
base-163-01-client
Open

feat(load-tests): generalize batch client for validity submissions#4398
refcell wants to merge 1 commit into
mainfrom
base-163-01-client

Conversation

@refcell

@refcell refcell commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Part of BASE-163 (validity/conditional load testing). Stack 1/5 — base: `main`.

Prepares the load tester to submit validity (conditional) transactions with no behavior change.

  • Add `base-execution-txpool` dependency and reuse its canonical `ValidityPredicate`/`ValidityOperator` types (no local wire mirror).
  • Introduce `SubmitItem` and `BatchRpcClient::send_transactions`, selecting the JSON-RPC method per element (`eth_sendRawTransaction` vs `base_sendRawTransactionValidity`). Mixed batches supported; responses correlate by id.
  • Keep `send_raw_only` as a plain-only wrapper; existing submission path routes through it unchanged.
  • Surface the JSON-RPC `error.code` via `BatchSendError` so method-not-found (-32601) is detectable downstream.

Tests: 7 new unit tests; `cargo test -p base-load-tests`, clippy, fmt all green.

🤖 Generated with Claude Code

@linear

linear Bot commented Aug 13, 2026

Copy link
Copy Markdown

BASE-163

@cb-heimdall

cb-heimdall commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 1/2
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 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 1
Sum 2

Comment on lines +212 to +218
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BatchSendError {
/// JSON-RPC error code, when the failure came from a server error object.
pub code: Option<i64>,
/// Human-readable error message.
pub message: String,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: BatchSendError implements Display but not std::error::Error. Since it's a public error type (re-exported from lib.rs), it should implement the Error trait for interoperability with the broader error ecosystem (e.g., anyhow, eyre, ? propagation chains).

Suggested change
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BatchSendError {
/// JSON-RPC error code, when the failure came from a server error object.
pub code: Option<i64>,
/// Human-readable error message.
pub message: String,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BatchSendError {
/// JSON-RPC error code, when the failure came from a server error object.
pub code: Option<i64>,
/// Human-readable error message.
pub message: String,
}
impl std::error::Error for BatchSendError {}

@refcell
refcell marked this pull request as ready for review August 13, 2026 01:13
@refcell refcell self-assigned this Aug 13, 2026
BrianBland
BrianBland previously approved these changes Aug 13, 2026
@refcell
refcell force-pushed the base-163-01-client branch from 5c5942c to 816c404 Compare August 13, 2026 20:49
@cb-heimdall
cb-heimdall dismissed BrianBland’s stale review August 13, 2026 20:49

Approved review 4930242586 from BrianBland is now dismissed due to new commit. Re-request for approval.

submitted += Self::record_submitted(&ctx, signed, hash, measured).await;
}
BatchSendResult::Error(msg) => match Self::classify_batch_error(msg) {
BatchSendResult::Error(err) => match Self::classify_batch_error(err.message) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: classify_batch_error(err.message) discards the structured code field. When validity submissions are routed through this path (future PRs in the stack), a method-not-found response (-32601) will be classified purely by string matching and land in BatchTxError::Rejected — a terminal rejection that won't be retried.

Consider passing the full BatchSendError into classify_batch_error (or at least checking err.is_method_not_found() before falling through to string classification) so the structured code is used when available. Not urgent for this PR since validity txs don't flow here yet, but worth keeping in mind for the next PR in the stack.

@refcell
refcell force-pushed the base-163-01-client branch from 816c404 to 3d2f83b Compare August 13, 2026 21:33
Prepare the load tester to submit validity (conditional) transactions
without changing current behavior.

- Add base-execution-txpool dependency and reuse its canonical
  ValidityPredicate/ValidityOperator types (no local wire mirror).
- Introduce SubmitItem and BatchRpcClient::send_transactions, which
  selects the JSON-RPC method per element: eth_sendRawTransaction for
  plain items and base_sendRawTransactionValidity for items carrying
  predicates. Mixed batches are supported; responses correlate by id.
- Keep send_raw_only as a plain-only convenience wrapper; the existing
  submission path now routes through it (behavior unchanged).
- Surface the JSON-RPC error code on batch errors via BatchSendError so
  method-not-found (-32601) is detectable by later routing work.
- Extract pure build_batch_body/parse_batch_body helpers and cover them
  with unit tests.

Part of BASE-163.

Co-Authored-By: Claude <noreply@anthropic.com>
@refcell
refcell force-pushed the base-163-01-client branch from 3d2f83b to 721ae04 Compare August 13, 2026 21:40
@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

This PR cleanly generalizes the batch RPC client to support validity (conditional) transaction submissions. The refactoring from String to BatchSendError, the introduction of SubmitItem, and the extraction of build_batch_body/parse_batch_body into testable helpers are well-done. Test coverage is solid with 7 new unit tests covering method selection, serialization, response correlation, and error display.

No new findings — the two existing inline comments (missing std::error::Error impl on BatchSendError, and classify_batch_error discarding the structured code field) are both valid and worth addressing. No additional issues found.

Not block-production-sensitive (load-test infrastructure only).

@github-actions

Copy link
Copy Markdown
Contributor

Base Std historical fork tests

Fork Result Passed Failed Skipped base/base base-anvil base-std
Beryl pass 616 0 13 7c6b0f5f 6d744e03 4658f1b7
Cobalt pass 721 0 14 7c6b0f5f ae7557c4 3f899009

View run

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.

3 participants