feat: implement TIP-1011 enhanced access key permissions#3384
feat: implement TIP-1011 enhanced access key permissions#3384
Conversation
legion2002
requested review from
0xKitsune,
dankrad,
danrobinson,
fgimenez,
howydev,
klkvr,
mattsse and
onbjerg
as code owners
✅ Changelog found on PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d11fad3e41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Co-authored-by: 0xrusowsky <90208954+0xrusowsky@users.noreply.github.com>
| } | ||
|
|
||
| /// Root-only create-or-replace updates for one or more target call scopes. | ||
| pub fn set_allowed_calls( |
There was a problem hiding this comment.
This seems valid, is there a reason we dont emit an event when scope is changed?
| if scopes.is_empty() { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Isnt this what the remaining logic is doing? If scopes is empty we are returning early and allowing any calls from the key. If scopes is not empty, we upsert the target scope and flips the is_scoped flag. Lmk if I am missing something though.
| let mut rules = Vec::new(); | ||
|
|
||
| for selector in selectors { | ||
| let recipients: Vec<Address> = self.key_scopes[key_hash].target_scopes[target] | ||
| .selector_scopes[selector] | ||
| .recipients | ||
| .read()? | ||
| .into(); | ||
|
|
||
| rules.push(SelectorRule { | ||
| selector, | ||
| recipients, | ||
| }); | ||
| } |
There was a problem hiding this comment.
nit: consider using collect, feel free to ignore though if preferred.
let rules = selectors
.into_iter()
.map(|selector| {
let recipients: Vec<Address> = self.key_scopes[key_hash].target_scopes[target]
.selector_scopes[selector]
.recipients
.read()?
.into();
Ok(SelectorRule { selector, recipients })
})
.collect::<Result<Vec<_>>>()?;
| && subject.matches_spending_limit_update(&updates.spending_limit_spends) | ||
| && let Some(ref mut provider) = state_provider | ||
| && exceeds_spending_limit(provider, subject, tx.transaction.fee_token_cost()) | ||
| && exceeds_spending_limit( |
There was a problem hiding this comment.
we will address this in a follow up tx pool PR
| "too many call scopes in key authorization", | ||
| ))); | ||
| } | ||
|
|
Stacks on #3384. Replaces nested `Option<Vec<_>>` key-scope fields with vec-based semantics for `selector_rules` and `recipients`, and canonicalizes empty nested lists to omitted trailing RLP fields while still decoding omitted/`0x80`/`0xc0` as allow-all. Updates the TIP, handler, txpool validation, and focused tests so empty selector lists mean address-only scope and empty recipient lists mean no recipient restriction.
| } | ||
| } | ||
|
|
||
| if spec.is_t3() { |
| keychain | ||
| .set_transaction_key(access_key_addr) | ||
| .map_err(|e| EVMError::Custom(e.to_string())) | ||
| .map_err(|e| EVMError::Custom(e.to_string()))?; |
👁️ Cyclops Security Review🧭 Auditing · mode=
⚙️ Controls
📜 3 events🔍 |
legion2002
added
cyclops
👁️ Cyclops Security Review🧭 Auditing · mode=
Findings
⚙️ Controls
📜 7 events🔍 |
👁️ Cyclops Security Review🧭 Auditing · mode=
⚙️ Controls
📜 3 events🔍 |
Co-Authored-By: 0xKitsune <77890308+0xKitsune@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d4ab7-a146-70d4-bcf2-8aa7032a9a54
Updates keychain gas estimation snapshots to match new values from TIP-1011 permission changes, and adds missing changelog entry to fix the CI changelog check. Prompted by: daniel Co-authored-by: 0xKitsune <77890308+0xKitsune@users.noreply.github.com>
The key_type mismatch check is gated behind spec.is_t3(), so the test needs a T3 EVM to exercise the rejection path. Co-Authored-By: 0xKitsune <77890308+0xKitsune@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d4ac0-09dc-7361-9cc6-035d3f173f9c
7 participants
Implements TIP-1011 end to end for T3 access keys. Adds periodic TIP-20 spending limits, per-target and per-selector call scopes, and recipient-constrained token calls across the spec, authorization payload, AccountKeychain precompile, runtime enforcement, and txpool admission.
The key-authorization payload now carries exact RLP and JSON semantics for
limitsandallowed_calls, including the distinction between unrestricted keys and deny-all scoped keys. The precompile persists the new restriction state, validates uniqueness and constrained-selector invariants, exposes the new allowed-call and spending-limit interfaces, hides stale scopes for revoked or expired keys, and keeps the T3 access-key creation restrictions intact.Handler and txpool validation now reject malformed TIP-1011 authorizations before execution and enforce selector and recipient scoping consistently with onchain storage. The diff also extends coverage for RLP roundtrips, periodic rollover, allowed-call normalization, recipient-constrained TIP-20 selectors, and access-key execution paths.