fix(eip8130): allow AuthorizeActor while locked when expiry outlives unlock by chunter-cb · Pull Request #4699 · base/base

chunter-cb · 2026-08-26T17:39:21Z

Summary

Mirror base/eip-8130#90: while locked, AuthorizeActor is permitted for a new add or expiry-only re-lease when the granted expiry outlives the unlock floor (now + delay hard-locked, unlocksAt pending unlock; expiry == 0 always passes).
Live actors keep frozen authenticator/scope/policy under lock; RevokeActor and delegation still revert AccountIsLocked; grants at or below the unlock floor revert ExpiryDoesNotOutliveUnlock.
Move lock enforcement from the config authorizer into the apply path so batch signature auth still runs while per-op lock policy is enforced at apply time.

Test plan

cargo test -p base-execution-eip8130 (202 tests)
Hard-locked: expiry at floor → ExpiryDoesNotOutliveUnlock; above floor / unbounded → lands
Locked re-lease: expiry-only succeeds; scope rewrite → AccountIsLocked
Locked revoke → AccountIsLocked; expired actor id treated as new add
Config authorizer still validates locked batch signatures (apply rejects revoke)

…unlock Mirror Keystore PR #90: locked accounts may add actors or re-lease expiry on live actors, while RevokeActor and identity/policy rewrites stay frozen.

cb-heimdall · 2026-08-26T17:39:25Z

✅ Heimdall Review Status

Requirement Status More Info

Reviews

✅ 1/1

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 0

Sum 1

…rize_op Structure the locked guard as keystore does: empty live slot is a new add; live slot allows expiry-only re-lease above the unlock floor.

github-actions · 2026-08-26T17:50:51Z

Review Summary

This PR moves lock enforcement from the ConfigChangeAuthorizer (batch-level auth) into the AccountChangeApplier (per-op apply) so that AuthorizeActor can proceed on locked accounts when the granted expiry outlives the unlock floor, while RevokeActor remains fully blocked. The change mirrors Keystore._applyAuthorize from the reference Solidity contract.

Architecture

The refactoring is well-structured:

Removing the blanket is_locked rejection from ConfigChangeAuthorizer::authorize_with_account_state is correct — batch signature auth must still run for locked accounts so that per-op lock policy can be enforced at apply time.
The new apply_authorize_op cleanly separates the JIT expiry skip, locked-account enforcement, and the actual authorize write.
enforce_locked_authorize_rules correctly implements the Solidity reference's "new add or expiry-only re-lease" semantics.
unlock_floor correctly handles both hard-locked (now + delay) and pending-unlock (unlocks_at) cases, with proper saturating_add and & 0xFFFF masking matching the uint16 delay storage width.
TxAuthError::AccountLocked is correctly retained for delegation lock enforcement in transaction.rs.

Error handling in block-production paths

The new ApplyError::AccountIsLocked and ApplyError::ExpiryDoesNotOutliveUnlock variants propagate through TxAuthError::Apply → BaseTransactionError::eip8130, which is an invalid-transaction error that causes the transaction to be skipped/rejected — not a fatal builder error. The txpool validator maps both variants to user-visible rejection reasons. No block-production halt/stall risk identified.

Test coverage

Thorough test coverage for:

Hard-locked: expiry at floor rejected, above floor accepted, unbounded (expiry == 0) accepted
Re-lease: expiry-only change succeeds, scope change rejected
Expired actor treated as new add
Pending unlock floor: at floor rejected, above floor accepted
Locked revoke rejected
Config authorizer still authenticates locked batch signatures

No findings

The implementation is correct, mirrors the reference contract, and has adequate test coverage. No issues identified.

github-actions · 2026-08-26T18:00:35Z

Base Std historical fork tests

Fork	Result	Passed	Failed	Skipped	base/base	base-anvil	base-std
Beryl	pass	616	0	13	`f4da5a74`	`cecddfa5`	`4658f1b7`
Cobalt	pass	721	0	14	`f4da5a74`	`fb00db40`	`e30b3421`

View run

github-actions · 2026-08-26T18:27:41Z

Caution

This PR may regress performance. 1 benchmark(s) slower by more than 10% beyond the noise band: batch_transaction_encoding/temporary_frame_buffers (+33.7%).

Benchmark results (advisory)

Median time on the PR head versus the base branch, measured on the same host. Wall-clock, so a change is only flagged when it clears ±10% and the confidence intervals do not overlap. Only benchmarks past the ±10% threshold (plus new or dropped ones) are listed. This check never blocks a merge.

Benchmark	Base	Head	Δ median
`batch_transaction_encoding/temporary_frame_buffers`	219.20 µs	293.07 µs	+33.7% ⚠️ slower
`tx_selection_predicate_rescan/transactions=10000_predicate_transactions=1000_predicates=1_stat`	7.17 ms	6.42 ms	-10.4% ✅ faster

47 benchmark(s) within ±10% omitted.

View run · Re-run benchmarks

The helper took 8 arguments, tripping clippy::too_many_arguments. Its single caller already has `locked`, `now`, and `is_unsequenced` in scope, so inline the JIT-expiry skip and locked-account guard directly into the AuthorizeActor match arm. Behavior is unchanged.

github-actions · 2026-08-26T19:13:34Z

✅ All benchmarks green — 14 within ±2% (deterministic instruction counts). View run

Benchmark details (14)

Benchmark	Base (target)	Head (this PR)	Δ instructions
`batch_queue/drain/drain_cached_span_batches`	242,027	242,027	+0.0%
`batch_transaction/encode_in_place/encode_in_place`	4,199,759	4,199,759	+0.0%
`batch_transaction/temporary_frame_buffers/temporary_frame_buffers`	8,408,350	8,408,350	+0.0%
`flashblock_decode/decode/brotli`	3,296,472	3,296,472	+0.0%
`flashblock_decode/decode/plain_json`	2,280,182	2,280,182	+0.0%
`flz/compress_len/real_contract_call`	43,148	43,148	+0.0%
`flz/compress_len/synthetic_0`	38,205	38,205	+0.0%
`flz/compress_len/synthetic_1`	54,682	54,682	+0.0%
`flz/compress_len/synthetic_2`	147,976	147,976	+0.0%
`flz/data_gas`	43,059	43,059	+0.0%
`flz/tx_estimated_size`	43,056	43,056	+0.0%
`frame_parse/decode/single_4kib`	1,031	1,031	+0.0%
`frame_parse/parse_frames/few_large`	1,053,062	1,053,062	+0.0%
`frame_parse/parse_frames/many_small`	154,763	154,763	+0.0%

github-actions · 2026-08-26T19:17:00Z

Review Summary

The PR moves EIP-8130 lock enforcement from batch-level authorization (ConfigChangeAuthorizer) to per-op enforcement in AccountChangeApplier, allowing AuthorizeActor while locked when the granted expiry outlives the unlock floor (expiry == 0 always passes), while still rejecting RevokeActor and delegation on locked accounts.

No critical or high-severity findings. The implementation is correct and well-tested. Specific observations:

Correctness

The enforce_locked_authorize_rules logic correctly distinguishes new adds (empty live config → allow with expiry check) from re-leases (live config → require authenticator/scope/policy match, expiry-only update).
The unlock_floor computation correctly mirrors Keystore._unlockFloor: hard-locked uses now + delay (with & 0xFFFF defense-in-depth matching the Solidity uint16() cast), pending-unlock uses the stored unlocks_at timestamp.
The resolve_live_actor_config expiry check (now > config.expiry) is consistent with the authorizer semantics (valid while now <= expiry), distinct from the JIT skip (config.expiry <= now) which is intentionally more aggressive.
JIT skip ordering before the locked check is correct: avoids false ExpiryDoesNotOutliveUnlock reverts for already-lapsed grants that would be silently dropped anyway.
Policy comparison for re-lease uses raw slot reads (get_policy_manager/get_policy_commitment), which correctly handles both explicit actor entries and inline self-actors (whose policy slots are zero).

Test Coverage

Good coverage of happy and error paths: hard-lock floor boundary (at vs above), unbounded expiry, revoke rejection, expiry-only re-lease, scope-change rejection, expired-actor-as-new-add, and pending-unlock floor behavior.
The updated locked_account_batch_still_authorizes_signature test correctly validates that batch signature auth still runs while per-op lock policy is enforced at apply time.

Block Production Impact

This code runs in the EIP-8130 transaction validation/execution path. The new ApplyError variants (AccountIsLocked, ExpiryDoesNotOutliveUnlock) propagate as TxAuthError::Apply, causing transaction rejection — not a fatal builder error. The txpool validator correctly maps both new variants to descriptive rejection reasons. No block-production halt/stall risk.

Minor Notes

IncrementLocalEpoch is intentionally not blocked while locked (consistent with the Solidity: it is a non-destructive rotation mechanism, not a configuration change). Consider adding a test asserting this is allowed for documentation purposes, but not required.
Delegation retains its own independent lock guard in transaction.rs:231-232, unaffected by this change.

LGTM — no blocking issues.

xenoliss · 2026-08-26T19:45:58Z

                    if is_unsequenced && config.expiry != 0 && config.expiry <= now {
                        continue;
                    }
+                    if locked {


This adds a moving deadline (expiry > now + delay) that WatchManifest does not track. A sequenced tx can become invalid without any watched state change and be retried on subsequent builds.

Agreed this is real, and it's a consequence of moving lock enforcement to apply-time — the now-relative predicate no longer flows into the WatchManifest the authorizer builds. A few notes on scope, and a proposed fix:

Consensus is unaffected. At inclusion the applier re-runs with the block's real now, so an op that has crossed the deadline reverts ExpiryDoesNotOutliveUnlock deterministically. This is purely a mempool-efficiency issue: the tx can linger and be retried across builds instead of being dropped when it lapses.

Only hard locks move. The deadline exists only while the account is hard-locked, where unlock_floor = now + delay. A pending unlock uses a fixed unlocks_at, so expiry <= unlocks_at is time-independent and doesn't drift.

Bounded expiry only. expiry == 0 always passes, so only bounded grants have a deadline: the op is valid while now < expiry - delay.

Proposed fix: fold that per-op deadline (min(expiry - delay) across the batch's hard-locked AuthorizeActor ops) into the manifest's effective_expiry, the same way sender/payer actor expiries already seed manifest_expiry / InvalidationKey::expiry_bucket in the validator — so the pool evicts the tx at the boundary rather than on a watched state change. That spans the applier → validator → manifest boundary and deserves its own tests, so I'd prefer to do it as a focused follow-up rather than expand this PR (happy to land it here instead if you'd rather — let me know). I'll open a follow-up issue/PR either way.

xenoliss · 2026-08-26T20:19:10Z

+            return Err(ApplyError::ExpiryDoesNotOutliveUnlock);
+        }
+
+        let current = storage.resolve_live_actor_config(account, actor_id, now)?;


This reads persisted storage and misses inline-self updates held in state from earlier ops in the same batch. It should resolve self against the evolving AccountState; otherwise a second identity/scope rewrite bypasses AccountIsLocked.

Fixed in c56387b. Added resolve_live_actor_config_with_state, which resolves the inline self from the evolving in-batch AccountState (the explicit actor_config and policy slots are written eagerly, so those are still read from storage). The locked guard now uses it, so an add-then-mutate of the inline self within a single locked batch is correctly caught as a live-entry identity change instead of reading as a fresh add. Added locked_self_release_uses_evolving_state_within_batch, which fails against the previous storage-based read.

xenoliss · 2026-08-26T20:27:16Z

-                    // verifying and estimation pipelines pass the real block
-                    // timestamp so the skip is applied identically.
+                    // Replayable JIT path: drop an already-lapsed grant without reverting.
                    if is_unsequenced && config.expiry != 0 && config.expiry <= now {


This differs from Solidity’s _isExpired, which uses now > expiry. I think we should align and use < now for parity.

Good catch — aligned with _isExpired. Fixed in c56387b: the JIT skip now uses a shared ActorConfig::is_expired (expiry != 0 && now > expiry), so a grant with expiry == now is still live for that second and installs rather than being dropped. resolve_live_actor_config uses the same helper for consistency, and the boundary test (apply_skips_lapsed_jit_grant) was updated to assert the expiry == now grant lands.

- Resolve the locked-authorize live-entry check against the evolving in-batch AccountState instead of persisted storage, so an add-then-mutate of the inline self within one locked batch cannot bypass AccountIsLocked. Adds a resolve_live_actor_config_with_state helper + regression test. - Align the JIT expiry skip with Keystore._isExpired (strict now > expiry) via a new ActorConfig::is_expired; a grant with expiry == now is still live for that second and installs. Updates the boundary test.

…uthorize # Conflicts: # crates/execution/eip8130/src/account_config.rs

github-actions · 2026-08-26T21:08:23Z

Review Summary

No issues found. The changes are well-structured and correctly implement the locked-account AuthorizeActor policy.

Key observations:

JIT skip boundary fix: Aligns with the canonical _isExpired boundary (now > expiry, strict) — previously expiry == now was incorrectly skipped; now it installs as live. Test updated accordingly.
Lock enforcement moved to apply path: Removing the blanket lock check from ConfigChangeAuthorizer and adding per-op policy in the apply loop enables AuthorizeActor (add/re-lease above unlock floor) while still rejecting RevokeActor and delegation. The delegation lock check remains in transaction.rs:226.
enforce_locked_authorize_rules correctly resolves the live config against the evolving in-batch AccountState (not stale storage) for inline-self actors, while reading policy from eagerly-written storage. The test locked_self_release_uses_evolving_state_within_batch directly covers this.
unlock_floor uses saturating_add to prevent overflow and & 0xFFFF to extract the delay for hard-locked accounts, matching the uint16 delay field.
New ApplyError variants are exhaustively handled in the txpool validator — no block-production risk since these are clean invalid-tx rejections for user-controlled inputs.
Test coverage is thorough: hard-lock, pending unlock, expiry-at-floor, unbounded expiry, re-lease (expiry-only), scope/authenticator change rejection, expired-actor-as-new-add, and batch-state evolution.

fix(eip8130): allow AuthorizeActor while locked when expiry outlives …

1ea9d44

…unlock Mirror Keystore PR #90: locked accounts may add actors or re-lease expiry on live actors, while RevokeActor and identity/policy rewrites stay frozen.

refactor(eip8130): consolidate locked AuthorizeActor into apply_autho…

fbcf4c5

…rize_op Structure the locked guard as keystore does: empty live slot is a new add; live slot allows expiry-only re-lease above the unlock floor.

xenoliss reviewed Aug 26, 2026

View reviewed changes

chunter-cb added 2 commits August 26, 2026 16:48

Merge remote-tracking branch 'origin/main' into feat/eip8130-locked-a…

f6701a0

…uthorize # Conflicts: # crates/execution/eip8130/src/account_config.rs

xenoliss approved these changes Aug 27, 2026

View reviewed changes

chunter-cb added this pull request to the merge queue Aug 27, 2026

Merged via the queue into main with commit 218b671 Aug 27, 2026
27 checks passed

chunter-cb deleted the feat/eip8130-locked-authorize branch August 27, 2026 16:07

Uh oh!

Conversation

chunter-cb commented Aug 26, 2026

Summary

Test plan

Uh oh!

cb-heimdall commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Heimdall Review Status

Uh oh!

github-actions Bot commented Aug 26, 2026

Review Summary

Architecture

Error handling in block-production paths

Test coverage

No findings

Uh oh!

github-actions Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Base Std historical fork tests

Uh oh!

github-actions Bot commented Aug 26, 2026

Benchmark results (advisory)

Uh oh!

github-actions Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 26, 2026

Review Summary

Correctness

Test Coverage

Block Production Impact

Minor Notes

Uh oh!

xenoliss Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

chunter-cb Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

xenoliss Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunter-cb Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

xenoliss Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

chunter-cb Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 26, 2026

Review Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cb-heimdall commented Aug 26, 2026 •

edited

Loading

github-actions Bot commented Aug 26, 2026 •

edited

Loading

github-actions Bot commented Aug 26, 2026 •

edited

Loading

xenoliss Aug 26, 2026 •

edited

Loading