pallet-revive: EIP-7702 (continued)#10936
Conversation
b1522d8 to
21967d3
Compare
|
/cmd bench --runtime dev --pallet pallet_revive |
| Some(new_hash) != old_code_hash | ||
| { | ||
| if let Err(e) = CodeInfo::<T>::increment_refcount(new_hash) { | ||
| log::warn!(target: LOG_TARGET, "increment_refcount({new_hash:?}) failed: {e:?}"); |
There was a problem hiding this comment.
shouldn't happen since we know that new_hash is the hash of an existing contract.
| Some(old_hash) != target_code_hash | ||
| { | ||
| if let Err(e) = CodeInfo::<T>::decrement_refcount(old_hash) { | ||
| log::warn!(target: LOG_TARGET, "decrement_refcount({old_hash:?}) failed: {e:?}"); |
There was a problem hiding this comment.
shouldn't happen since we decrement the code_hash of the previous contract
| *delegate_target = None; | ||
| if !contract_info.code_hash.is_zero() { | ||
| if let Err(e) = CodeInfo::<T>::decrement_refcount(contract_info.code_hash) { | ||
| log::warn!(target: LOG_TARGET, "decrement_refcount({:?}) failed: {e:?}", contract_info.code_hash); |
| origin, | ||
| amount, | ||
| Some(exec_config), | ||
| )?; |
There was a problem hiding this comment.
if charge_deposit or refund_deposit fail, the previous elements of authorization_list are not reverted because this is executed outside the transaction context (as stated in a comment in eth_call). Do I understand this correctly?
Either way, if one authorization fails then all of them or part of them are not executed.
Should these also be continue?
Maybe wrap each authorization in a with_transaction?
There was a problem hiding this comment.
so first we process the authorisations then the transaction execute, if any authorisation fails because we don't have enough gas then everything revert. This should not happen in practice cause validating the transaction ensure we have enough gas to pay for the worst case.
| } | ||
|
|
||
| if !account_exists { | ||
| Pallet::<T>::charge_deposit(None, origin, &account_id, ed, exec_config)?; |
There was a problem hiding this comment.
| /// Returns the net deposit change. | ||
| /// | ||
| /// Note: the target's `code_hash` is snapshotted at delegation time. This is fine | ||
| /// because `set_code` (the only way to change a contract's code) requires root. |
There was a problem hiding this comment.
If root calls set_code on a contract that is used a delegate then its code is changed.
But the refcount is not updated, i.e. old codehash has refcount but new codehash does not.
set_delegation: increment refcount on old_hash.
set_code: old_hash becomes new_hash in the contract that is delegated to.
clear_delegation: decrement refcount on new_hash
We would end up with an orphaned ref and possible underflow on refcount[new_hash]?
There was a problem hiding this comment.
If you clear_delegation that will decrement the refcount on the old_hash the delegated contract is still pointing to, so that should be fine.
You could say that, then it's not great cause your delegated contract is now pointing to something different than the contract, but since that's the root operation, that should not happen outside of dev, (unless this is done through governance, but I don't see why one would do that)
| new_accounts: result.new_accounts, | ||
| existing_accounts: result.existing_accounts, | ||
| }); | ||
| result.weight_refund = worst_case_weight.saturating_sub(actual_weight); |
There was a problem hiding this comment.
This doesn't refund skipped authorizations. So if any authorization was skipped we overcharge.
There was a problem hiding this comment.
overcharging if you have added garbage should be fine.
| transaction_encoded: Vec<u8>, | ||
| effective_gas_price: U256, | ||
| encoded_len: u32, | ||
| authorization_list: Vec<evm::AuthorizationListEntry>, |
There was a problem hiding this comment.
We should cap the size of this vec
There was a problem hiding this comment.
Not sure if that's necessary, since eth_call is created through the eth_transact
There was a problem hiding this comment.
I agree, gas limit would make this low risk.
On the other hand: a BoundedVec would be a cheap insurance against future call path without gas validation.
| } | ||
| } | ||
| StorageDeposit::Refund(refund) | ||
| }) |
There was a problem hiding this comment.
What happens if the DelegatedEOA writes some storage (and pays deposit) and then clears delegation? Will that deposit be locked forever? will some other account be able to clear it?
There was a problem hiding this comment.
no one will be able to clear it until they restore a delegation with a contract that let you free that storage.
There was a problem hiding this comment.
The EOA owns the storage so it just needs to delegate to a contract that allows to clear those specific storage keys?
There was a problem hiding this comment.
if you clear the delegation and delegate call to your address, it's like doing a delegate call to a regular EOA, so it essentially does nothing since there are no code anymore to delegate to
There was a problem hiding this comment.
I am confused. If a non-contract account creates a delegation to a contract account, then the non-contract account becomes the EOA account right?
Then it writes some storage and clears the delegation. Then the only way for the EOA to retrieve its deposit is to re-create the same delegation and make the delegate clear the storage? Or is there another way?
|
closed in favor of #12229 |
Continuation of #10851.
evm-test-suite PR paritytech/evm-test-suite#141
This PR implements EIP-7702 ("Set EOA Account Code") for
pallet-revive, enabling Externally Owned Accounts (EOAs) to designate a contract whose code will be executed on their behalf when they are called.What is EIP-7702?
EIP-7702 lets an EOA sign an authorization that says "when someone calls me, execute contract X's code". When a transaction includes these authorizations, any subsequent calls to the delegated EOA will execute the target contract's code: but using the EOA's own storage and balance. This enables smart account patterns (batching, gas sponsorship, etc.) without requiring account abstraction (ERC-4337) or proxy contracts.
For background:
Changes
Pallet integration
The
eth_calldispatchable now accepts and processes an authorization list before executing the call.Note: Updating this dispatchable's signature should be safe: it is not dispatched directly by users but is the inner call of
eth_transact, which is signed by an Ethereum wallet and submitted viaeth-rpc.Authorization processing
The
Transaction7702type (with itsauthorization_list) was already defined and parsed: this PR adds the actual processing logic.process_authorizationsvalidates each entry (chain ID, signature, nonce, account type), creates the authority account if needed, and sets or clears the delegation. Invalid authorizations are silently skipped per spec.Key semantics:
Revive-specific complexity: In Revive the per-authorization pre-dispatch weight/gas is higher than EVM because:
Storage changes
A new
AccountType::DelegatedEOAvariant is introduced:delegate_target: Option<H160>: the contract address this EOA delegates to.contract_info: ContractInfo: storage accounting (child trie, base deposit) for delegated EOAs.Key behaviors:
EOAtoDelegatedEOApermanently.delegate_target = Nonebut the account staysDelegatedEOA.Execution changes
During a call, the runtime resolves delegation:
delegate_target, code is loaded from the target contract but execution uses the EOA's storage.Benchmarks
process_new_account_authorization(n): worst case: creates N new accounts with delegations.process_existing_account_authorization(n): best case: processes N authorizations for existing accounts (used for weight refund calculation).RPC integration
eth-rpcsupports submitting and dry-running EIP-7702 transactions.eth_getCodereturns the0xef0100 || addressprefix for delegated EOAs (per EIP-7702 spec).