-
Notifications
You must be signed in to change notification settings - Fork 1k
EIP-3607 added check to make sure a contract account cannot transfer funds as an EOA account #9717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
/cmd prdoc --audience runtime_dev --bump patch |
…time_dev --bump patch'
substrate/frame/revive/src/exec.rs
Outdated
@@ -1527,6 +1527,22 @@ where | |||
Ok(()) | |||
} | |||
|
|||
// Only reject if the account actually has deployed contract code (non-empty code hash). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EIP-3607: Reject transactions from senders with deployed code
Do not allow transactions for whichtx.sender
has any code deployed
We need to deny all pallet::call to be dispatched by a contract.
Easiest is probably to replace
let origin = ensure_signed(origin)?;
by a wrapper function that also checks it's not a contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this additional check go in every extrinsic?
#[pallet::call]
impl<T: Config> Pallet<T>
Only in the call
extrinsic?
In all extrinsics that currently have ensure_signed
?
EDIT
Replace all ensure_signed
calls with the new wrapper check?
All GitHub workflows were cancelled due to failure one of the required jobs. |
/// Ensure the origin has no code deplyoyed if it is a signed origin. | ||
fn ensure_non_contract_if_signed(origin: &OriginFor<T>) -> Result<(), DispatchError> { | ||
use crate::exec::EMPTY_CODE_HASH; | ||
if let Ok(who) = ensure_signed(origin.clone()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit use ?
use crate::exec::EMPTY_CODE_HASH; | ||
if let Ok(who) = ensure_signed(origin.clone()) { | ||
let addr = <T::AddressMapper as AddressMapper<T>>::to_address(&who); | ||
if let Some(contract) = AccountInfo::<T>::load_contract(&addr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we just need to check load_contract(...).is_none() ?
@@ -1193,6 +1213,15 @@ where | |||
storage_deposit_limit: DepositLimit<BalanceOf<T>>, | |||
data: Vec<u8>, | |||
) -> ContractResult<ExecReturnValue, BalanceOf<T>> { | |||
if let Err(contract_result) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want that in bare_instantiate as well
maybe refactor ensure_non_contract_if_signed so it return a Result<(), ContractResult>, so we don't have to repeat the mapping twice
No description provided.