Skip to content

Support indexing pallet-assets approval (and permit) #281

Description

@0xRVE

[pallet-assets] ERC-20 Approval log mirror for allowance changes

Follow-up to #12581.

Context

#12581 makes substrate-side pallet-assets balance changes visible to Ethereum log consumers: AssetsCallback gained balance-change hooks, and Erc20TransferLogsCallback in pallet-assets-precompiles projects them into canonical ERC-20 Transfer logs at the asset's precompile address. The ERC20 precompile's own Transfer self-emit was removed in the same change, so there is exactly one Transfer log per balance movement regardless of which path caused it.

Allowances were left out of that scope. They change on several substrate-side paths with no log at all, so Approval still originates only from the ERC20 precompile. An indexer reconstructing ERC-20 state from logs therefore gets correct balances and incomplete allowances — it can observe a delegate spending an allowance (the resulting Transfer is mirrored) without ever having seen the grant that authorized it.

Current state

Path Effect on stored allowance ERC-20 log today
approve_transfer extrinsic (assets/src/lib.rs:1667) accumulates (+= amount) none
cancel_approval extrinsic (assets/src/lib.rs:1692) removes none
force_cancel_approval extrinsic (assets/src/lib.rs:1727) removes none
transfer_approved extrinsic decrements, removes at zero Transfer (mirrored), no Approval
fungibles::approvals::Mutate::approve (assets/src/impl_fungibles.rs:341) accumulates none
fungibles::approvals::Mutate::transfer_from (assets/src/impl_fungibles.rs:351) decrements Transfer (mirrored), no Approval
ERC20.approve precompile (assets/precompiles/src/lib.rs:546-585) sets (cancel, then approve) Approval (precompile self-emit)
ERC20.permit precompile (assets/precompiles/src/lib.rs:700-760) sets (cancel, then approve) Approval (precompile self-emit)
ERC20.transferFrom precompile decrements Transfer, no Approval
destroy_approvals (assets/src/lib.rs:1036) batch-removes up to RemoveItemsLimit none

Relevant asymmetry, introduced deliberately by #11279: the native path accumulates (approved.amount.saturating_add(amount), assets/src/functions.rs:976) while both EVM entry points implement ERC-20 set semantics by cancelling any existing approval before re-approving.

Why this is not a copy of the balance hooks

The balance mirror could hook the shared internal (assets/src/functions.rs:759) and delete the precompile's self-emit, because one precompile transfer is one pallet-level transfer. Neither half of that holds for approvals.

  1. One EVM approve is two pallet-level approval operations. Over a non-zero existing allowance, the precompile calls do_cancel_approval then do_approve_transfer. Hooks on those internals would produce Approval(owner, spender, 0) followed by Approval(owner, spender, new), on top of the precompile's own Approval(owner, spender, value) — three logs and a spurious intermediate zero for one logical set.
  2. Hook placement has no free choice. do_approve_transfer is reached from the extrinsic, from fungibles::approvals::Mutate::approve, and directly from the precompile. Hooking the internal double-emits on the precompile path; hooking the dispatchables misses the fungibles trait path. (approvals::Mutate currently has no in-tree consumer beyond its own impl, so dispatchable-level hooks are the cheaper option today, at the cost of leaving that path unhooked for external runtimes.)
  3. The hook argument is a delta, but Approval.value is a total. On the accumulate path the hook would receive amount, not the resulting allowance, so the mirror needs a post-state Approvals::get(..).amount read rather than forwarding its argument.

Open design questions

  • Keep or remove the precompile self-emit. Unlike Transfer, it may be correct to keep it and instead ensure the mirror does not fire on precompile-originated calls — the precompile is the only caller that knows its two pallet operations are one logical set. If it is removed instead, the same atomic-cutover constraint as #12581 applies: self-emit removal and mirror wiring must ship in one runtime upgrade, or a block range emits duplicate or zero Approval logs.
  • Infinite-allowance value divergence. ERC20.approve saturates storage at Balance::MAX but emits Approval carrying the raw call.value (assets/precompiles/src/lib.rs:514-517). A mirror reading post-state would emit Balance::MAX where the precompile emits type(uint256).max, so the two sources disagree for the standard infinite-allowance idiom. Needs a decision on which value is canonical.
  • Whether to mirror the transferFrom decrement. EIP-20 mandates Approval only on approve; implementations differ on transferFrom (older OpenZeppelin emitted, current does not), and the precompile's transfer_from does not. Not mirroring it keeps parity with the precompile and with current mainstream ERC-20 behaviour, at the cost of log-derived allowances drifting above their true value.
  • destroy_approvals has no bounded projection. Mirroring it means up to RemoveItemsLimit Approval(_, _, 0) logs per call — the same objection that rules out mirroring destroy_accounts. Probably a documented non-goal: log consumers must drop a token's allowances when the asset is destroyed.
  • force_cancel_approval has no ERC-20 analogue. Representable as Approval(owner, spender, 0), but no ERC-20 contract produces an allowance reset the owner did not initiate. Worth stating explicitly for consumers rather than leaving it to be inferred.
  • Weights. New hooks add a storage read plus a log emit to the approval extrinsics; /cmd bench regeneration is required before production, for both pallet_assets and any consumer reaching allowances through the fungibles traits.

Acceptance criteria

  • Allowance state is reconstructible from ERC-20 logs alone for every substrate-side path that changes it, except paths explicitly documented as non-goals.
  • Exactly one Approval log per logical allowance change, with no spurious intermediate zero from the precompile's reset-then-set emulation.
  • Approval.value carries the resulting allowance, with the infinite-allowance case resolved consistently between the mirror and the precompile.
  • Tests covering: the extrinsic path, the fungibles trait path (or an explicit decision to leave it unhooked), the precompile path asserting no duplicate Approval, and the type(uint256).max saturation case.
  • Weights regenerated; prdoc updated to remove the allowance scope caveat added by #12581.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions