Fix/relay selectors and pool tags

[nol4lej]

Summary

Two independent security fixes, plus the tests and structure that keep them from regressing.

Both share a failure mode worth naming: they were silent. The relay was rejecting every call it was supposed to accept, and the pool was admitting duplicates of the same spend. Neither surfaced an error, and the existing tests stayed green throughout.

pallet-shielded-pool — pool admission tagged one entry per nullifier

and_provides(x) contributes exactly one tag, so passing it a Vec encoded the whole nullifier set plus the relayer into a single blob. Three consequences, each free for an attacker since the fee is only charged on execution:

• Reordering the two inputs minted a second admissible entry for the same spend.
• Two transfers sharing only one note (A+B and A+C) did not collide at all, so one note could back unboundedly many entries.
• Transfer and unshield used different prefixes, so the same note could back one of each at the same time.

Every variant propagates and is revalidated network-wide while at most one can ever execute.

The fix calls and_provides once per nullifier, under a shared ShieldedPoolSpend namespace. Dummy nullifiers (all-zero) are excluded: they carry no identity, and tagging them would collide every padded single-input spend with every other.

relayer deliberately leaves the tag. Binding it made a copy with a swapped fee recipient a separate entry, so anyone could rebroadcast another user's spend pointed at their own account. Keyed on the nullifier the two are mutually exclusive, so taking the fee requires out-bidding — which means paying it.

Admission policy, not state transition. Consensus is unaffected. Nodes running the old logic keep accepting the duplicate variants, so the mitigation only completes as the network updates.

fc-rpc relay — the selector whitelist was stale for both operations

The client held 0x47fc44a2 (unshield) and 0x8c0f5d24 (privateTransfer), while the decoder answers to 0x4e505348 and 0x66ed2cd4.

Derived by keccak, the stale pair turn out to be real selectors from signatures two versions old — they froze when the signatures gained parameters. Relaying was therefore rejecting every call as unsupported selector, indistinguishable from a legitimate rejection. The same stale literals sat in the runtime's fallback list and in ts-tests/test-relay-rpc.ts, so the tests stayed green while testing nothing.

Both constants are now re-exported from pallet_evm_precompile_shielded_pool::selectors rather than copied, so the whitelist and the decoder have a single definition and cannot drift. A test still pins them to keccak of the ABI signatures, which the re-export alone does not guarantee.

Two more fixes in the same path:

• Per-operation calldata minimums were both 228 bytes, the shared head up to the fee slot. Past that the layouts diverge: unshield's head is 10 slots (324 with the selector), privateTransfer's is 8 (260). A call between 228 and its real minimum passed validation and reached the decoder truncated.
• gas_price and the fee word now saturate instead of panicking. U256::as_u128() panics above 2^128. The fee word is caller-controlled over an unauthenticated RPC, so one crafted 32-byte value took down the handler. gas_price comes from the runtime and is not attacker-reachable, but a panic there still kills the relay RPC.

Node-side only. No consensus effect, no state-transition change.

Testing

Every fix is pinned by mutation: the code was broken on purpose and the tests had to catch it.

Mutation	Caught by
and_provides over the whole Vec (the original bug)	4 tests
Separate tag prefix per operation	1 test
relayer back in the tag	2 tests
Dummy-nullifier filter removed	1 test
saturating_mul → wrapping_mul in the gas floor	1 test
Size cap > → >=	3 tests
Per-operation length gate removed	3 tests
Fee comparison shifted by one	6 tests
Whitelist check removed	1 test

One gap was found this way and closed. amount + fee in unshield's solvency check had no test on its overflow branch — AMOUNT_OVERFLOW appeared only in the constants test. Both operands are attacker-chosen and summed before the balance compare, and a sum that overflows comes out small, which passes. Four tests now cover it: the overflow itself, the solvency boundary (< not <=), the fee counting against the pool, and per-asset isolation.

New coverage:

• 9 adversarial relay tests — a whitelist full of unimplemented selectors, one padded with 4096 duplicates, gas-floor arithmetic at u128 boundaries, a zero governance fee, the fee one planck below the floor, unshield's selector on privateTransfer-length calldata, and the size cap at exactly the boundary.
• 9 precompile decoder tests — truncation at every offset, self-referential and u256::MAX offsets, offsets above u32 that look benign, huge array counts that must not allocate, oversized u32 slots, a maximal fee word, and two fuzz sweeps that must never panic.
• 4 pool solvency tests, above.

private_transfer_rejects_truncated_input had itself drifted — it used 0x8c0f5d24 and passed only because a wrong selector is rejected anyway, so it was testing nothing about truncation. It now takes the selector from the decoder.

Refactor: client/rpc/src/relay

validation.rs was 763 lines — 151 of code, 610 of tests — and its own header claimed "no runtime dependencies" while holding MAX_FEE_PER_GAS_WEI and RELAY_GAS_LIMIT, parameters of the transaction the relay signs rather than of the calldata it inspects. mod.rs was both the module root and the RPC implementation.

Split by what each part needs in order to run, not by topic:

File	Responsibility
config.rs	Constants by role: target, admission limits, tx parameters, Runtime API fallbacks
validation.rs	Pure checks — bytes in, verdict out (~75 lines)
rpc.rs	OrbinumRelay: dry run, nonce, signing, pool submission
mod.rs	Declarations and re-exports (~40 lines)
tests/	validation.rs and adversarial.rs

Keeping validation free of chain state is what makes the adversarial tests cheap: hostile calldata, no node, no async runtime.

This also surfaced a real bug: pallet-evm-precompile-shielded-pool had been added to [dev-dependencies] instead of [dependencies], so it linked for tests but not for the lib.

Not included

No runtime version bump. The relay_config() fallback fix changes a value compiled into the WASM, so publishing this will need spec_version 8 → 9 — deliberately left out of this PR.

transaction_version stays at 2 either way: no dispatch signature changes, so offline-signed extrinsics remain valid and wallet and runtime do not have to ship together. No migration, no state-transition change.

Follow-up

The ts-sdk still carries 0x1ec439cf for privateTransfer and encodes an extra ovkBlob argument. Against this node, every private transfer over the EVM path is rejected. That is a separate change, tracked outside this PR — but it means the SDK must land before the EVM transfer path works end to end.

rpc.rs — dry run, nonce allocation, signing, pool submission — has no tests, as it needs a mock client and pool. The optimistic nonce under Mutex is the part most exposed to concurrency bugs.

Verification

cargo test --release --lib --all      # 0 failures
cargo clippy --all-targets -- -D warnings
cargo fmt --all --check
taplo fmt --check

• pallet-shielded-pool — 347 pass
• pallet-evm-precompile-shielded-pool — 85 pass
• fc-rpc — 113 pass