Skip to content

feat(eth): whitelist EVM method selectors for rpcCall, runtime-configurable via admin API#1597

Open
pragmaxim wants to merge 7 commits into
masterfrom
enhancement/whitelisting-evm-read-call-method-selectors-for-rpc-call
Open

feat(eth): whitelist EVM method selectors for rpcCall, runtime-configurable via admin API#1597
pragmaxim wants to merge 7 commits into
masterfrom
enhancement/whitelisting-evm-read-call-method-selectors-for-rpc-call

Conversation

@pragmaxim

Copy link
Copy Markdown
Contributor

Closes #1593.

Motivation

Trezor Suite (Yield, Trading) needs to read ERC-20 allowance(address owner, address spender) across many different token contracts before showing or performing approve/deposit flows. <network>_ALLOWED_RPC_CALL_TO alone cannot express that — it would require whitelisting every token contract address individually.

What this PR does

1. Method-selector allowlist for rpcCall

New option ALLOWED_EVM_CALL_METHODS: a comma-separated list of 4-byte EVM method selectors (optional 0x prefix, case-insensitive) that websocket rpcCall requests may invoke on any address when the calldata starts with an allowed selector — e.g. 0xdd62ed3e for allowance(address,address).

  • Combines with ALLOWED_RPC_CALL_TO: a call is allowed when either its target address or its calldata selector is allowed; with neither configured, rpcCall stays unrestricted (unchanged behavior).
  • Fail-closed selector extraction: the whole data field is strictly hex-decoded (0x/0X prefix required, even length, ≥ 4 bytes), matching the backend node's own parsing — malformed calldata never matches a selector, so it cannot bypass the allowlist.
  • Fail-fast configuration: a malformed selector, or a set value that parses to no entries at all, is a configuration error.

2. Both allowlists are dynamic runtime settings

Rather than remaining static env vars read once at startup, both ALLOWED_RPC_CALL_TO and ALLOWED_EVM_CALL_METHODS are implemented as runtime settings, manageable through the internal server's authenticated admin API (existing HTTP Basic auth, BB_ADMIN_USER/BB_ADMIN_PASSWORD; a bearer-token scheme for a future JS admin UI can later wrap the same middleware):

GET    /admin/runtime-settings/<KEY>   → {"key":…,"value":…,"source":"db|env|unset"}
POST   /admin/runtime-settings/<KEY>   --data '{"value":"0xdd62ed3e,0x70a08231"}'
DELETE /admin/runtime-settings/<KEY>   → removes the override, reverts to the env default
  • Persistence: overrides are stored in RocksDB under dedicated cfDefault keys (JSON envelope with an updated-timestamp), independent of the periodically stored internalState blob. They survive restarts and take precedence over the env vars, which act as startup defaults only. Older Blockbook versions ignore the rows (env applies again after a rollback) and keep them intact.
  • Live effect: the effective allowlists are an immutable snapshot behind an atomic pointer on InternalState — the rpcCall hot path stays lock-free, and changes apply to the next call with no restart. Initialization runs in both the public-websocket and internal-server constructors (CAS-guarded), so any deployment topology gets correct values.
  • Write semantics: validate → store to DB → publish to live state → log. Invalid values return 400 and change nothing; a DB write failure returns 500 and leaves the live allowlists untouched, so a 200 always means the change is both durable and live. Every change is logged with old/new value, source and client address.
  • Un-restricting safely: POST {"value":""} (exactly empty) explicitly unconfigures a dimension — the only way to un-restrict at runtime while the env var has a value. Whitespace/separator-only values are rejected, so a botched automation input cannot silently disable the allowlist. DELETE validates the env fallback before removing the override, so a malformed environment cannot brick the next restart.
  • A minimal /admin/runtime-settings page shows the current values and their sources, with curl hints.

Behavior change (documented in docs/env.md)

The ALLOWED_RPC_CALL_TO parser is unified with the selector parser: entries are trimmed, empty entries skipped, and a set-but-empty value now fails startup. Previously "," silently allowlisted the empty string — which matched rpcCall requests with an empty to field — and "a, b" stored " b", which never matched.

Testing

  • Unit tests for both parsers, selector extraction fail-closed cases, and the allow-check matrix (address hit, selector hit, both, neither, malformed calldata).
  • DB-layer tests incl. empty-value-vs-missing-row distinction and persistence across a DB reopen.
  • Handler tests: GET/POST/DELETE happy paths, validation failures, auth gating, store-failure → 500 with untouched live state (fault-injection seam), restart resolution of a stored-empty override beating a set env var.
  • End-to-end test: a websocket rpcCall rejected under the env-seeded allowlist starts passing right after an authenticated admin POST (no restart), the override wins over env on a simulated restart, and DELETE reverts it.

go vet clean; full -tags unittest suite passes (728 tests across db, common, server).

🤖 Generated with Claude Code

pragmaxim and others added 3 commits July 2, 2026 05:34
Add <network>_ALLOWED_EVM_CALL_METHODS, a comma-separated list of 4-byte
method selectors that permits websocket rpcCall requests to any address
when the calldata starts with an allowed selector (e.g. 0xdd62ed3e for
ERC-20 allowance). Complements ALLOWED_RPC_CALL_TO: a call passes when
either its target address or its selector is allowed; with neither set
rpcCall stays unrestricted. Malformed calldata never matches (full hex
decode, fail closed) and malformed or empty selector config fails
startup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Turn ALLOWED_RPC_CALL_TO and ALLOWED_EVM_CALL_METHODS into runtime
settings: the environment variables remain the startup defaults, while
an override written through the authenticated internal API

  GET/POST/DELETE /admin/runtime-settings/<KEY>

is persisted in RocksDB (own cfDefault rows, independent of the
periodically stored internalState blob and invisible to older
versions), survives restarts, takes precedence over the environment
and takes effect on websocket rpcCall immediately, without a restart.

The live allowlists are an immutable snapshot behind an atomic pointer
on InternalState, so the rpcCall hot path stays lock-free and both
servers (public websocket, internal admin) share one view regardless
of which of them a deployment runs. Writes are strictly ordered:
validate (400 on invalid input) -> store to DB (500 and untouched live
state on failure) -> publish snapshot -> log the change with old/new
value, source and client address. POST of an exactly empty value
explicitly unconfigures a dimension; whitespace- or separator-only
values are rejected so a botched input cannot silently un-restrict
rpcCall; DELETE reverts to the environment default and validates it
first so a malformed environment cannot brick the next restart.

The TO-list parser is unified with the selector parser: entries are
trimmed, empty entries skipped and a set-but-empty value fails startup
(previously "," allowlisted the empty string, which matched rpcCall
requests with an empty to field).

A minimal /admin/runtime-settings page lists the current values and
their sources with curl hints.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Avoids allocating a decoded copy of the full calldata (up to 4 MiB) on
every rpcCall allowlist check. The full hex encoding is still validated
byte-by-byte to preserve the existing fail-closed behavior for malformed
calldata.
@pragmaxim

Copy link
Copy Markdown
Contributor Author
 1. Error message casing (server/runtime_settings.go:126): Capital I in "Invalid EVM call method selector..." — should be "invalid" to match lower-case convention (e.g.
    parseAllowedRpcCallTo's "%s is set but contains no addresses").

 2. Trim asymmetry (server/runtime_settings.go:142-158): resolveRuntimeSetting returns the raw os.Getenv value untrimmed, while updateRuntimeSetting trims POSTed values. After restart or
    DELETE, the admin page may show leading/trailing whitespace from env vars. Suggest trimming in resolveRuntimeSetting.

 3. Map key format (common/internalstate.go:139-153): To map keys include 0x prefix, Methods map keys don't. Each dimension is internally consistent, but worth documenting explicitly on the
    struct fields so a third allowlist dimension picks a convention.

 4. Guard asymmetry (server/internal.go:114): initRpcCallAllowlists and runtime-settings routes are gated behind ChainEthereumType in internal.go but not in websocket.go:178. The handlers are
    fully generic — consider moving them outside the guard for consistency. The internal-data-errors and contract-info routes should stay gated (those are EVM-specific).

 5. Interface completeness (server/runtime_settings.go:217-220): runtimeSettingStore has Store/Delete but no Get — readers must cast to *db.RocksDB. Consider adding GetRuntimeSetting and
    updating the failingRuntimeSettingStore mock.

 6. Release note: ALLOWED_RPC_CALL_TO="," (empty entries) will now fail at startup since the new parser rejects zero-address configs — previously this would allowlist empty-to requests. Any
    deployment relying on this will need updating.

@pragmaxim

Copy link
Copy Markdown
Contributor Author

API unification review: runtime-settings vs contract-info

The new /admin/runtime-settings/ endpoint introduces a new pattern for mutating admin state via JSON. The pre-existing /admin/contract-info/ is the only other admin endpoint that does this. Below are the inconsistencies between them.


1. contract-info write response is double-encoded

File: server/internal.goupdateContracts

return "\"{\\\"success\\\":\\\"Updated \" + strconv.Itoa(len(contractInfos)) + \" contracts\\\"}\"", nil

This returns a Go string containing pre-serialized JSON. The jsonHandler wrapper then marshals it again via json.Marshal, producing a double-quoted JSON string on the wire instead of a JSON object. The new endpoint avoids this by returning a typed struct (runtimeSettingResponse).

Fix: Make updateContracts return a proper struct and delete the pre-serialized string pattern.


2. Write request body format differs

Endpoint Body
POST /admin/contract-info/ Raw []ContractInfo
POST /admin/runtime-settings/<KEY> {"value":"..."}

The contract-info body IS the data; the runtime-settings body wraps it in a value key. If there is no strong reason for the wrapper, either:

  • Unwrap runtime-settings to accept the raw value string as the body, or
  • Add a thin abstraction so all future admin mutation endpoints use a consistent body shape.

3. No DELETE in the old API

contract-info has no way to remove a single entry. The old approach is "overwrite by posting an empty array". The new endpoint introduces a proper DELETE verb with env-var fallback semantics. Consider adding DELETE support to contract-info for symmetry, or document the divergence.


4. Key normalization differs

Endpoint Key extraction
contract-info/<address> r.URL.Path[i+1:] — raw, no normalization
runtime-settings/<KEY> strings.ToUpper(strings.TrimSpace(...)) — normalized

A user interacting with both APIs would see different behavior for the same path segment input. Recommend either:

  • Normalize the contract-info path segment too (lowercase the address), or
  • Extract a common helper urlPathSegment(r *http.Request) string used by both.

5. Route registration is gated inconsistently within the codebase

Both routes are registered inside a ChainEthereumType guard in internal.go, but the runtime-settings handlers (apiRuntimeSetting, runtimeSettingsPage) are fully generic — they contain no EVM-specific logic. The only EVM-specific part is the parseAllowedEvmCallMethods definition in runtimeSettingDefs, which would simply never produce matches on non-EVM coins (no EVM calldata to check). Meanwhile, initRpcCallAllowlists is called unconditionally in websocket.go:178 but conditionally in internal.go:114.

Fix: Move initRpcCallAllowlists and the two runtime-settings routes outside the ChainEthereumType guard in internal.go. Keep internal-data-errors and contract-info gated (those are genuinely EVM-specific).


6. Admin nav link gating mismatch

The "Runtime Settings" link in static/internal_templates/index.html is inside {{if eq .ChainType 1}}, matching the route registration guard. If fix #5 is applied, this if must be removed from the runtime-settings link (but kept for internal-data-errors and contract-info).


Summary of actionable items

# What Where Notes
1 Return a typed struct from updateContracts server/internal.go Fixes double-encoding bug
2 Align write body format server/runtime_settings.go or internal.go Optional — pick one convention
3 Add DELETE to contract-info server/internal.go Optional, for symmetry
4 Normalize contract-info path segment server/internal.go Or extract shared helper
5 Move routes outside ChainEthereumType guard server/internal.go Low risk, handlers are generic
6 Remove ChainType guard from runtime-settings nav link static/internal_templates/index.html Must follow #5

pragmaxim and others added 2 commits July 2, 2026 10:20
Addresses the API unification review on PR #1597:

- POST /admin/contract-info/ returns a typed {"updated":N} object instead
  of a pre-serialized string that jsonHandler double-encoded on the wire;
  a malformed body is now a 400 (public APIError) rather than a 500
- apiContractInfo dispatches on method like apiRuntimeSetting: POST/PUT
  write the collection path only (an address segment is rejected instead
  of silently ignored), unknown methods are a 400 instead of falling into
  the GET path
- new DELETE /admin/contract-info/<address> purges cached contract
  metadata (DB row + LRU entry) so it is re-fetched from the backend on
  the next read; idempotent, missing row reports deleted:false
- shared urlPathSegment helper unifies path-segment extraction; contract
  addresses get no case normalization (the chain parser is the authority),
  runtime-setting keys keep their uppercasing at the call site
- runtime-settings routes and initRpcCallAllowlists move out of the
  ChainEthereumType guard (the mechanism is chain-generic and the init
  already runs unconditionally in NewWebsocketServer); contract-info and
  internal-data-errors stay EVM-gated, nav links follow the same split

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smaller items from the PR #1597 review:

- lowercase the "invalid EVM call method selector" parse error to match
  the sibling parser's message convention
- trim environment values wherever they are resolved (startup and the
  DELETE fallback), so stray whitespace from an env file never surfaces
  in values; a set but whitespace-only env value is a configuration
  error rather than unset — treating it as unset would silently
  un-restrict rpcCall
- document the differing To/Methods map key formats on RpcCallAllowlists
  so a future allowlist dimension picks a convention deliberately
- runtimeSettingStore gains GetRuntimeSetting; resolveRuntimeSetting
  reads through the interface instead of requiring *db.RocksDB

Admin runtime-settings page gets an inline editing UI (edit/save/delete
per setting, replacing the curl-only instructions) and the page/nav/
routes are consistently available on all chain types.

Hardening found by review of the new DELETE /admin/contract-info/:

- bump protocolGen before the cache purge in DeleteContractInfoForAddress
  so a concurrent GetContractInfo cannot re-insert the deleted row into
  the LRU (same idiom as SetErcProtocol); without it the purge could be
  silently undone until eviction or restart
- DELETE returns (and logs) the purged record: the backend re-fetch
  restores only name/symbol/decimals, not the sync-owned
  createdInBlock/destructedInBlock, so the response gives the operator
  a POST-back restore path and the docs now state exactly what is
  discarded
- a failed save in the admin UI keeps the row in edit state instead of
  stranding an orphaned input, and error responses are unwrapped to
  their message

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pragmaxim pragmaxim requested a review from cranycrane July 2, 2026 18:13
pragmaxim and others added 2 commits July 3, 2026 04:36
Supports the bbctl counterpart (trezor/bbctl#10) of the runtime-settings
admin API in a two-replica deployment where each replica owns its DB:

- GET /admin/runtime-settings/ (bare collection path) returns every
  setting with its effective value and source as a JSON array, so a
  management tool reads the whole state per replica in one request;
  other methods on the bare path stay a 400
- initRpcCallAllowlists warns at startup when a stored override shadows
  a different (or malformed) environment value, making env/DB drift —
  a replica that missed an admin update, or an env change rolled out
  while an override exists — visible; the shadowed value is only
  compared, never parsed, so drift cannot fail a restart
- docs/env.md explains the deployment roles of the two sources: the env
  var is the deploy-managed baseline that survives a replica resync
  (the DB is wiped, and unset allowlists would silently un-restrict
  rpcCall), the stored override is the runtime layer that survives
  restarts until the next deploy ships an updated environment

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /admin/contract-info/ (bare collection path) lists the stored
records, mirroring the runtime-settings list convention. Unlike runtime
settings the collection is unbounded — sync stores a record per contract
creation, millions on a busy chain — so the list is paginated:
?limit=<1..10000> (default 1000) with a from cursor, and the response
{"contracts":[...],"next":"<address>"} carries the from of the next
page. ListContractInfos iterates cfContracts in address-descriptor
order (the CF holds only plain addrDesc keys; protocol rows live in
their own CF).

The former 400 for GET of the bare path moves to the list; DELETE and
POST semantics on the bare path are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

Allow whitelisting EVM read call method selectors for rpcCall

1 participant