refactor EAM interface #1032
refactor EAM interface #1032
Conversation
actors/eam/src/lib.rs
Outdated
| params: Create3Params, | ||
| ) -> Result<Create3Return, ActorError> { | ||
| // we only allow accounts to call this | ||
| rt.validate_immediate_caller_type(&[Type::Account, Type::EthAccount])?; |
There was a problem hiding this comment.
Let's change this to "accept any" and leave a comment that we validate later.
|
@vyzo I've made my changes. This just needs some tests now. |
|
TODO:
|
|
@vyzo passing this back to you. |
Stebalien
force-pushed
the
next-eam-refactor
branch
3 times, most recently
from
1af720b to
90febe0
Compare
| @@ -38,3 +38,7 @@ pub mod evm { | |||
|
|
|||
| pub const RESURRECT_METHOD: u64 = 2; | |||
| } | |||
|
|
|||
| pub mod account { | |||
| pub const PUBKEY_ADDRESS_METHOD: u64 = 2; | |||
There was a problem hiding this comment.
Even if this is a built-in actor and this authorised to call a reserved method, why not call the FRC42 endpoint?
There was a problem hiding this comment.
Why? This is internal code, why bother with the user space methods? We can use reserved methods just fine at this level.
There was a problem hiding this comment.
This is a "trusted" call (we need the target to actually be an account) so I'd keep using the private one for now.
| } | ||
| } | ||
|
|
||
| fn resolve_caller_external(rt: &mut impl Runtime) -> Result<(EthAddress, EthAddress), ActorError> { |
There was a problem hiding this comment.
We might have to deal with multisigs a here?
There was a problem hiding this comment.
We don't currently allow them, we expect EthAccount or Account; we'll have to be explicit about those.
But yes, we might want to allow multisig ownership of a newly created contract, but that's a can of worms we can open later.
There was a problem hiding this comment.
CreateExternal needs to happen from an account. We could introduce an additional "create native" (or something like that), but, IMO, that's not critical right now.
We're also trying to encourage users to switch over to EVM multisigs.
actors/eam/src/lib.rs
Outdated
| let addr = resolve_eth_address(rt, caller_id)?; | ||
| Ok((addr, addr)) | ||
| } | ||
| _ => Err(ActorError::forbidden(format!("disallowed caller code cid {}", caller_code_cid))), |
There was a problem hiding this comment.
Let's specify the caller type in the error message for better UX/DX.
There was a problem hiding this comment.
At the cost of a syscall to get the type? I don't think that's the right thing to do, aiding debuggability and UX is fine, but this will cost gas.
The user can always lookup the code CID on his own to find the type.
There was a problem hiding this comment.
@vyzo we already have the type.
That way, the encoding is identical to the invoke params. This will make signature validation simpler.
… data propagation test fix situation with placeholders vs ethaccounts undo test_vm changes
vyzo
force-pushed
the
next-eam-refactor
branch
from
08d7c72 to
f5e417d
Compare
|
everything (cept the borked coverage job) green. |
See filecoin-project/ref-fvm#1434
These are the core code changes; once we are happy with them I'll have to fix the tests and replace all create invocations with create_external in lotus side.