Skip to content

refactor(account-keychain): prototype vec-backed scope indexes#3370

Closed
legion2002 wants to merge 9 commits intotanishk/tip-1011-implfrom
tanishk/tip-1011-impl-vec-scopes
Closed

refactor(account-keychain): prototype vec-backed scope indexes#3370
legion2002 wants to merge 9 commits intotanishk/tip-1011-implfrom
tanishk/tip-1011-impl-vec-scopes

Conversation

@legion2002
Copy link
Copy Markdown
Contributor

@legion2002 legion2002 commented Mar 30, 2026

Stacks on #3323.

Prototypes the alternative storage shape discussed in review: vec-backed target and selector indexes for external enumeration, with mapping-backed payload rows preserved as the internal source of truth for validation.

What changed:

  • replace Set<address> targets with Vec<address>
  • replace Set<u32> selectors with Vec<bytes4>
  • store selector keys directly as bytes4 / FixedBytes<4> instead of round-tripping through u32
  • update scoped-call intrinsic gas accounting and TIP text to match the vec-backed write pattern
  • keep the TIP diff limited to scope-specific changes only

Write-path synchronization:

  • validation no longer scans vec indexes in the hot path
  • target_scopes and selector_scopes are the internal source of truth
  • small mutator helpers now keep the vec indexes synchronized on add/remove so getAllowedCalls() and other external enumeration remain correct
  • added a regression covering selector-index replacement without duplicate externally returned rules

Audit outcome:

  • I ran an oracle review on the rebased diff to look for stale-state hazards, gas-accounting drift, and abstraction needs
  • the conclusion was that a small local sync layer on the mutator path is the right tradeoff here, and that a larger generic storage abstraction is not needed for this PR

Validation:

  • cargo test -p tempo-precompiles account_keychain --lib
  • cargo test -p tempo-transaction-pool keychain --lib

Stacks on tanishk/tip-1011-impl to show the alternative storage shape discussed in PR #3323. Replaces the enumerable-set indexes for targets and selectors with vec-backed enumeration while keeping the mappings as the hot-path source of truth, and stores selector keys as FixedBytes<4> instead of round-tripping through u32.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
Restores selector-list membership checks on the T3 validation path, switches fresh vec-backed scope indexes to push-based writes, and updates scoped-call intrinsic gas accounting to match the new storage pattern. Adds a regression test for stale selector rows that are not present in the vec index.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
@legion2002 legion2002 force-pushed the tanishk/tip-1011-impl-vec-scopes branch from d855492 to 6aa1cc6 Compare March 30, 2026 15:32
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

⚠️ Changelog not found.

A changelog entry is required before merging. We've generated a suggested changelog based on your changes:

Preview
---
tempo-precompiles: patch
tempo-primitives: patch
tempo-revm: patch
---

Replaced `Set` with `Vec` for target and selector membership in account keychain call-scope storage, making validation fail-closed on vec membership. Updated gas accounting to reflect reduced slot writes (4→3 per scope entry) and added tests for rejection when targets or selectors are missing from the vec index.

Add changelog to commit this to your branch.

Updates the TIP to match the rebased prototype branch: consolidated spending-limit rows, vec-backed target and selector indexes, bytes4 selector keys, concrete nested slot offsets, and the revised scoped-call intrinsic gas formula.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
@legion2002 legion2002 force-pushed the tanishk/tip-1011-impl-vec-scopes branch from 6aa1cc6 to 2c2a34f Compare March 30, 2026 15:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

📊 Tempo Precompiles Coverage

📦 Download full HTML report

Adds small local helpers to centralize Vec+Mapping index invariants for targets and selectors, makes validation reject off-index rows explicitly, adds a stale-target regression test, clarifies raw-protocol vs mutator normalization for empty selector rule lists, and trims the TIP back to scope-specific documentation changes only.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
Removes the local SelectorKey alias and uses FixedBytes<4> directly in the vec-backed scope prototype.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
Removes the small local helper functions for vec-backed target and selector guards and inlines the logic at the call sites, preserving the same fail-closed behavior for off-index rows.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
Removes target and selector vec membership checks from the validation path, adds dedicated write-path helpers to keep vec indexes synchronized with target_scopes and selector_scopes for external enumeration, and replaces the stale-index regressions with a replacement test that checks selector index synchronization across updates.

Amp-Thread-ID: https://ampcode.com/threads/T-019d3ef3-f5d5-708c-9d23-0e4558c5bebf
@legion2002
Copy link
Copy Markdown
Contributor Author

Closed in favour of #3410

@legion2002 legion2002 closed this Apr 1, 2026
legion2002 added a commit that referenced this pull request Apr 1, 2026
## Stack Context

This is the middle PR in the TIP-1011 stack.
- Parent: [#3384](#3384)
`feat(primitives): add TIP-1011 wire format and ABI`
- Child: [#3386](#3386)
`feat(node): enforce TIP-1011 restrictions`

## What This PR Does

This PR implements TIP-1011 inside the `AccountKeychain` precompile:
- periodic spending-limit state and reset behavior
- allowed-call target / selector / recipient scoping
- precompile mutators and readers for the new restriction state
- hardfork-gated dispatch and direct readers that depend on the new
layout

This is the PR where the spec becomes concrete precompile and storage
behavior. It intentionally stops short of handler and txpool runtime
enforcement, which lands in PR3.

## How To Review

Review this like a smart-contract PR.

1. Start in `crates/precompiles/src/account_keychain/mod.rs` and verify
the storage model and invariants: uniqueness, rollover behavior,
normalization, selector and recipient validation, and any assumptions
about reads and writes.
2. Look for efficiency opportunities as you go: slot count, tight
packing, unnecessary writes, and whether the chosen layout is the right
tradeoff for expected access patterns.
3. Check access gating carefully for the important mutators, especially
`authorizeKey`, `setAllowedCalls`, `removeAllowedCalls`, and
`updateSpendingLimit`.
4. Review `crates/precompiles/src/account_keychain/dispatch.rs` for ABI
routing and fork gating.
5. As a final pass, think ahead to how the handler will interact with
these precompile functions and where they will be used in
[#3386](#3386).
6. Related context: [#3370](#3370)
is a draft PR that adds an enumerable map utility and makes the
`targets` / `target_scopes` path more efficient. It is worth keeping in
mind when evaluating this layout.

## Note

If you want the full end-to-end diff in one place, see
[#3323](#3323). It is useful for
seeing how the ideas interact across the stack, and Cyclops also runs
there.
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