Skip to content

docs(rpc): improve eth rpc docs#275

Merged
mattsse merged 1 commit intomainfrom
matt/docs-eth
Nov 28, 2022
Merged

docs(rpc): improve eth rpc docs#275
mattsse merged 1 commit intomainfrom
matt/docs-eth

Conversation

@mattsse
Copy link
Collaborator

@mattsse mattsse commented Nov 28, 2022

@mattsse mattsse requested a review from rkrasiuk November 28, 2022 11:21
@mattsse mattsse added the A-rpc Related to the RPC implementation label Nov 28, 2022
@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #275 (e9f5d61) into main (0e436ae) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #275   +/-   ##
=======================================
  Coverage   72.59%   72.60%           
=======================================
  Files         228      228           
  Lines       20674    20674           
=======================================
+ Hits        15008    15010    +2     
+ Misses       5666     5664    -2     
Impacted Files Coverage Δ
crates/net/rpc-api/src/eth.rs 0.00% <ø> (ø)
crates/net/rpc/src/eth/eth_server.rs 0.00% <0.00%> (ø)
crates/net/discv4/src/lib.rs 68.59% <0.00%> (+0.29%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mattsse mattsse merged commit b30e824 into main Nov 28, 2022
@mattsse mattsse deleted the matt/docs-eth branch November 28, 2022 13:16
Copy link

@kartik-ritual kartik-ritual left a comment

Choose a reason for hiding this comment

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

Cross-Repo PR Review: CLI Agent Precompile (0x080C) Containerization

Reviewed as part of coordinated 7-PR set. Design docs: https://github.com/ritual-net/arshan-agent-sessions/tree/arshmain/sessions/25-02-19-agent-cli-precompile-refactor

cc @kartik-ritual @spencerperkins


CRITICAL #1 — Delivery Result ABI Mismatch (cross-repo with executor-go-internal #99)

Zero CLI Agent Phase 2 settlements can succeed.

extract_agent_metrics_from_delivery() (and the block verifier's extract_agent_metrics) expects the delivery result to follow the agentcall.Phase2Response ABI layout:

(uint8 version, bool success, string response, string stopped_reason, uint16 iterations, uint16 tool_calls, string error_message)

It reads bytes 128-191 as uint16 iterations and uint16 tool_calls.

But the executor-go CLI Agent handler (response.go:55-126) encodes CLIAgentOutput as:

(bool success, string error, string text_response, tuple[] artifacts)

At bytes 128-191 of this encoding, Reth reads dynamic string offset data as uint16 → garbage or None → settlement silently dropped at payload builder stage (payload/src/lib.rs:2725-2740).

Verified against code on arsh/agentic-flow: No extract_cli_agent_metrics function exists anywhere in the Reth codebase. The defined CLI_AGENT_ITERATION_FEE_WEI, CLI_AGENT_TOOL_CALL_FEE_WEI, and CLI_AGENT_MAX_TOOL_CALLS constants in constants.rs are dead code — never referenced in any fee calculation.

Fix: Either (a) add iterations and tool_calls fields to CLIAgentOutput.Encode() in executor-go, or (b) create a separate extract_cli_agent_metrics_from_delivery() in Reth that understands the CLI Agent ABI layout. Option (a) is simpler. @arshan-ritual to decide.


CRITICAL #3 — async_pool Deadline Merge Regression

async_pool.rs:515-538 computes max_poll_block for 8 precompile types (FHE, HTTP, AGENT_CALL, IMAGE, AUDIO, VIDEO, AUTONOMOUS_AGENT, ZK_TWO_PHASE) but CLI_AGENT_PRECOMPILE is missing. It falls through to the else { None } arm at line 536.

A dead extract_max_poll_block_cli_agent() function exists at line 71 but is never called. Root cause: merge regression from commit 9ce222c2e9 (CKKS merge) dropped the CLI_AGENT arm that was added in 1004042001. The subsequent merge b068a7439c partially restored CLI_AGENT support but missed this call site.

Impact: NOT unbounded memory growth (Phase 1 TTL still cleans up). The real impact is premature eviction — Phase 2 deadline is always longer than Phase 1 TTL, so the pool evicts CLI Agent transactions before the executor can deliver results → Phase 2 settlement fails silently. Liveness bug.

Fix: Add CLI_AGENT_PRECOMPILE to the match arm at line 515 and call extract_max_poll_block_cli_agent().


HIGH #4 — Escrow Uses Wrong max_tool_calls Constant

wallet.rs calculates CLI Agent escrow using AGENT_MAX_TOOL_CALLS=100 instead of CLI_AGENT_MAX_TOOL_CALLS=500:

} else if precompile_address == AGENT_CALL_PRECOMPILE
    || precompile_address == CLI_AGENT_PRECOMPILE
{
    agent_escrow_lockup(AGENT_MAX_ITERATIONS, AGENT_MAX_TOOL_CALLS) // Uses 50, 100

CLI agents make far more tool calls than orchestrated agent calls (design doc says up to 500). Under-escrows by 5×, meaning executors may not get paid for actual work.

Fix: Branch on CLI_AGENT_PRECOMPILE separately and use the CLI-specific constants.


HIGH #7 — PR Title is Misleading

Title says "fix: remove duplicate HTTP_CALL_PRECOMPILE import" but this is a 745-line feature PR adding:

  • New cli_agent.rs ABI codec (428 lines)
  • Fee constants and escrow logic
  • Block builder and verifier changes (consensus-critical)
  • Mempool deadline tracking

Please update the title to accurately reflect the scope, e.g. feat: add CLI Agent Precompile (0x080C) support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rpc Related to the RPC implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants