Skip to content

feat(eip8130): enshrined authenticator dispatch (Authenticate step)#3467

Draft
chunter-cb wants to merge 2 commits into
hh/eip-8130-canonical-addressesfrom
hh/eip-8130-authenticator-dispatch
Draft

feat(eip8130): enshrined authenticator dispatch (Authenticate step)#3467
chunter-cb wants to merge 2 commits into
hh/eip-8130-canonical-addressesfrom
hh/eip-8130-authenticator-dispatch

Conversation

@chunter-cb

Copy link
Copy Markdown
Contributor

Summary

Adds base-execution-eip8130, the first piece of the EIP-8130 stateful validation pipeline that works for both mempool admission and block inclusion. It implements step 2 of the validation flow — Authenticate — as a stateless, pure function:

given a signing hash and an authentication blob (authenticator || data), route to the named authenticator, verify the signature, and return the resolved actorId.

The canonical authenticator set is enshrined as native Rust, keyed by the canonical CREATE2 addresses from Eip8130Contracts (the registry added in #3440, which this stacks on):

Authenticator actorId Notes
native secp256k1 (ECRECOVER sentinel, EOA path) bytes20(recovered) raw ecrecover; accepts v ∈ {0,1,27,28}, any s
K1_AUTHENTICATOR contract bytes20(recovered) mirrors OZ ECDSA.recover: requires v ∈ {27,28}, rejects high-s
P256_AUTHENTICATOR keccak256(x‖y) low-s enforced (OZ P256.verify)
WEBAUTHN_AUTHENTICATOR keccak256(x‖y) mirrors OZ WebAuthn.verify, requireUV = false
DELEGATE_AUTHENTICATOR bytes20(delegate) depth-1; see below

REVOKED_AUTHENTICATOR and any non-canonical address are hard-rejected.

⚠️ Enshrined ≠ precompile (design note)

This is a protocol fast-path, not an EVM precompile. It does not shadow the authenticator addresses: ordinary EVM CALL/STATICCALL to those addresses still execute the real deployed contract bytecode (e.g. AccountConfiguration.verifySignature() / wallet code / non-8130 chains). The native code here is invoked only by the protocol's own AA validation path.

Consequence: the native implementation MUST produce byte-identical actorId results to the deployed contracts. The enshrined logic is pinned to a contract version via the *_INIT_CODE_HASH constants in Eip8130Contracts — a bytecode change shifts the canonical address (caught by the registry drift test) and requires re-validating parity here. A differential parity test against the deployed contracts (via the EVM) is a planned follow-up.

Scope (what this PR is not)

This crate is stateless: no storage reads, no EVM. The stateful Authorize step is layered on top in the next PR — actor_config lookup, the implicit-EOA rule, scope/expiry, and (for delegate) authorizing the nested actor against the delegated account's config in SIGNATURE context.

For the delegate authenticator, dispatch verifies the nested signature and returns a DispatchOutcome::Delegated { delegate_account, nested_authenticator, nested_actor_id, .. } obligation for the authorize stage to discharge against state.

Open question for review

The registry lists both the native ECRECOVER sentinel (address(1)) and the deployed K1_AUTHENTICATOR contract. The spec's canonical k1 is the native sentinel, but the deployed K1 contract uses OZ ECDSA.recover (stricter v, low-s). This PR routes them to different semantics to stay faithful to each. If we want a single k1 behavior, that should be decided before block-validation wiring.

Test plan

  • cargo test -p base-execution-eip8130 — 15 self-consistent crypto + reject-path tests (ecrecover/K1 strictness divergence, high-s rejection, P-256, WebAuthn challenge-binding, delegate obligation + depth-1/non-canonical rejection, revoked/non-canonical/malformed-length rejects)
  • cargo clippy -p base-execution-eip8130 --all-targets -- -D warnings
  • cargo fmt --check
  • (follow-up) differential parity test vs deployed authenticator contracts via EVM

Stacked on #3440 (Eip8130Contracts registry).

Add `base-execution-eip8130`, a stateless crate implementing step 2
("Authenticate") of the EIP-8130 validation flow: route a signing hash +
authentication blob to the canonical authenticator, verify the signature, and
return the resolved actorId.

Enshrines the canonical authenticator set as native Rust keyed by the
`Eip8130Contracts` CREATE2 addresses:
- native secp256k1 ecrecover (EOA / ECRECOVER sentinel; raw semantics)
- K1 authenticator contract (OZ ECDSA.recover: v in {27,28}, reject high-s)
- P-256 raw (low-s enforced; actorId = keccak256(x||y))
- WebAuthn (mirrors OZ WebAuthn.verify, requireUV = false)
- Delegate (depth-1; verifies nested signature and surfaces the nested-actor
  authorization obligation for the stateful authorize stage)

Enshrinement is a protocol fast-path, not a precompile: it does not shadow the
authenticator addresses, so ordinary EVM calls still hit the deployed contracts.
The native logic must stay byte-identical to those contracts (pinned via the
registry init_code_hash constants); a differential parity test is a follow-up.

15 self-consistent crypto + reject-path tests.
Comment thread crates/execution/eip8130/Cargo.toml Outdated
Comment on lines +17 to +19
# crypto
k256.workspace = true
p256.workspace = true

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.

Production code imports k256::ecdsa::{RecoveryId, Signature, VerifyingKey} and p256::ecdsa::{Signature, VerifyingKey, signature::hazmat::PrehashVerifier}, which require the ecdsa feature on both crates. Currently ecdsa is only enabled in [dev-dependencies] (line 34-35), not here in [dependencies].

This compiles today only because Cargo feature unification merges the ecdsa feature from other workspace crates (e.g. base-common-precompiles, base-proof-tee-registrar). If those crates are ever removed, or if this crate is compiled in a minimal workspace / published standalone, the build will fail.

Suggested change
# crypto
k256.workspace = true
p256.workspace = true
k256 = { workspace = true, features = ["ecdsa"] }
p256 = { workspace = true, features = ["ecdsa"] }

Comment on lines +138 to +146
/// P-256 raw authenticator. `data = r(32) || s(32) || x(32) || y(32) || pre_hash(1)`.
/// `actorId = keccak256(x || y)`.
fn p256(hash: B256, data: &[u8]) -> Result<B256, AuthError> {
if data.len() != 129 {
return Err(AuthError::MalformedAuth);
}
let (r, s, x, y) = (&data[0..32], &data[32..64], &data[64..96], &data[96..128]);
Self::p256_verify(hash.as_slice(), r, s, x, y)?;
Ok(keccak256([x, y].concat()))

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.

The doc comment says data = r(32) || s(32) || x(32) || y(32) || pre_hash(1) and the length check enforces exactly 129 bytes, but data[128] (the pre_hash byte) is never read or validated. Two concerns:

  1. Parity risk: if the deployed P256 authenticator contract actually inspects this byte (e.g. to select prehash mode), the enshrined path silently ignores it, which could produce divergent accept/reject behavior between the EVM and native paths.

  2. Dead input: if the byte is genuinely unused by the contract too, consider documenting that it's a padding/reserved byte that must be present but is ignored, so future readers don't wonder whether it's a bug.

Comment on lines +234 to +241
/// Narrows a `clientDataJSON` index from `uint256` to `usize`, rejecting
/// implausibly large values.
fn index(value: &U256) -> Result<usize, AuthError> {
if *value > U256::from(u32::MAX) {
return Err(AuthError::MalformedAuth);
}
Ok(value.as_limbs()[0] as usize)
}

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.

The guard *value > U256::from(u32::MAX) allows values up to u32::MAX (4 GiB), but then extracts with value.as_limbs()[0] as usize. On a 32-bit target, usize is 32 bits and as_limbs()[0] is a u64, so values between u32::MAX + 1 and u64::MAX that pass the check would silently truncate. This isn't a practical concern for this crate (it won't run on 32-bit), but for robustness you could use:

usize::try_from(value.as_limbs()[0]).map_err(|_| AuthError::MalformedAuth)

which is correct on all platforms and eliminates the separate range check entirely.

Comment thread crates/execution/eip8130/Cargo.toml Outdated
base64.workspace = true

# alloy
alloy-primitives = { workspace = true, features = ["k256"] }

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.

The k256 feature on alloy-primitives enables secp256k1 key/signature integration (e.g. alloy_primitives::Signature::recover_address_from_prehash). This crate doesn't use any of that — it does ecrecover directly via the k256 crate. Unless this feature is needed transitively by alloy-sol-types decoding, consider dropping it:

Suggested change
alloy-primitives = { workspace = true, features = ["k256"] }
alloy-primitives.workspace = true

- Enable the `ecdsa` feature explicitly on `k256`/`p256` in [dependencies]
  instead of relying on workspace feature unification (the dev-dependency
  duplicates are now redundant and removed).
- Drop the unused `k256` feature on `alloy-primitives` (ecrecover is done via
  the `k256` crate directly).
- Document that the P-256 `pre_hash` byte (data[128]) is reserved: the deployed
  contract requires its presence but never reads it, and we mirror that for
  parity.
- Make `index()` pointer-width-correct via `usize::try_from` on all targets.
@chunter-cb

Copy link
Copy Markdown
Contributor Author

Addressed the review in 1b2c41887:

  • Must fix — ecdsa feature in [dependencies]: enabled features = ["ecdsa"] on both k256 and p256 in [dependencies] so the crate no longer relies on workspace feature unification. Removed the now-redundant [dev-dependencies] duplicates. Verified the crate builds and all 15 tests pass with the explicit features.
  • P-256 pre_hash byte: confirmed against the deployed P256Authenticator — it enforces length == 129 but never reads data[128] (it's carried only so the contract wire format matches the native-authenticator form). Our impl already matched this exactly; added a doc comment marking the byte as reserved/ignored with a parity note to re-pin if the contract ever starts interpreting it.
  • Nit — index() 32-bit: switched to usize::try_from(*value), which is correct on all targets and drops the manual range check.
  • Nit — alloy-primitives k256 feature: dropped; ecrecover is done via the k256 crate directly. Confirmed it wasn't needed transitively (builds clean without it).

cargo test, clippy --all-targets -- -D warnings, and fmt --check all green.

@github-actions

Copy link
Copy Markdown
Contributor

Review Summary — base-execution-eip8130

Thorough review of the new crate implementing EIP-8130 authenticator dispatch (Authenticate step). The cryptographic implementations (secp256k1 ecrecover, K1/OZ-strict, P-256, WebAuthn, Delegate) are correct and well-structured. The code is clean, idiomatic, and the error handling is solid.

Prior inline comments (still relevant)

The four inline comments from the previous review run cover the significant findings:

  1. k256/p256 ecdsa feature must be in [dependencies], not only [dev-dependencies] — build currently relies on Cargo feature unification from other workspace crates. This is the most important fix.
  2. alloy-primitives k256 feature appears unnecessary — the crate does ecrecover directly via the k256 crate, not through alloy_primitives::Signature.
  3. P256 data[128] (pre_hash byte) is present but never read — now documented in the code comments (lines 141-146), which is satisfactory.
  4. U256-to-usize index narrowing — now fixed with usize::try_from (line 245).

Additional observations (no new inline comments)

  • WebAuthn verification is correct: challenge binding, type check, UP flag, backup-state consistency (BS=1 → BE=1), and the P-256 signature over sha256(authData || sha256(clientDataJSON)) all match the OZ WebAuthn.verify contract.
  • Delegate depth-1 enforcement is sound: double-guarded by both the explicit nested_authenticator == DELEGATE_AUTHENTICATOR check in delegate() and the allow_delegate=false parameter propagated to authenticate_inner.
  • Revoked/non-canonical rejection paths are tested and correct.
  • The #[non_exhaustive] on AuthenticatorDispatch (unit struct) is appropriate for forward-compatibility. Consider also adding it to AuthError and DispatchOutcome if these types may gain variants before stabilization, but this is optional for an internal crate.

Verdict

The crate is well-implemented. The ecdsa feature issue (#1 above) should be fixed before merge to avoid a latent build breakage. The other inline comments are lower priority.

@github-actions

Copy link
Copy Markdown
Contributor

✅ base-std fork tests: all 616 passed

base/base is fully in sync with the base-std spec.

Dependency Ref Commit
base-std main 9b246ae9
base-anvil 85f5a0e2867cb88c55e9af29b348511f91cbc383 85f5a0e2

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.

1 participant