feat: Partial implementation of FIP-0048 (f4/delegated addresses) #1118
Conversation
arajasek
force-pushed
the
asr/delegated-addresses
branch
from
3895554
to
93fb73b
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1118 +/- ##
==========================================
- Coverage 89.28% 89.07% -0.22%
==========================================
Files 93 94 +1
Lines 19611 19718 +107
==========================================
+ Hits 17510 17564 +54
- Misses 2101 2154 +53
|
actors/init/src/lib.rs
Outdated
| if existing { | ||
| // NOTE: this case should be impossible, but we check it anyways just in case something | ||
| // changes. | ||
| return Err(ActorError::forbidden("cannot exec over an existing actor".into())); |
There was a problem hiding this comment.
Include the colliding ID for debugging.
runtime/src/runtime/mod.rs
Outdated
| @@ -74,6 +74,10 @@ pub trait Runtime: Primitives + Verifier + RuntimePolicy { | |||
| /// If the argument is an ID address it is returned directly. | |||
| fn resolve_address(&self, address: &Address) -> Option<ActorID>; | |||
|
|
|||
| /// Looks-up the "delegated" address of an actor by ID, if any. Returns None if either the | |||
| /// target actor doesn't exist, or if the target actor doesn't have either an f4 address. | |||
There was a problem hiding this comment.
| /// target actor doesn't exist, or if the target actor doesn't have either an f4 address. | |
| /// target actor doesn't exist, or doesn't have an f4 address. |
runtime/src/test_utils.rs
Outdated
| @@ -458,6 +464,17 @@ impl<BS: Blockstore> MockRuntime<BS> { | |||
| self.id_addresses.insert(source, target); | |||
| } | |||
|
|
|||
| pub fn add_delegated_address(&mut self, source: Address, target: Address) { | |||
There was a problem hiding this comment.
Just take an ActorID as the source parameter. Change the maps to also use ActorID type in the appropriate parameter so it's obvious which way round they go.
Consider calling this set_delegated_address().
runtime/src/test_utils.rs
Outdated
| @@ -111,6 +112,8 @@ pub struct MockRuntime<BS = MemoryBlockstore> { | |||
| pub miner: Address, | |||
| pub base_fee: TokenAmount, | |||
| pub id_addresses: HashMap<Address, Address>, | |||
| pub delegated_addresses: HashMap<Address, Address>, | |||
| pub delegated_addresses_source: HashMap<Address, Address>, | |||
There was a problem hiding this comment.
I don't think this second map is needed - these mappings should just go in the id_addresses map instead. These mappings should just replicate what's in the Init actor. I can understand that we need the ID -> delegated address map explicitly because we don't have the actor state tree, which would otherwise encapsulate it.
There was a problem hiding this comment.
I think you're right, will make this change -- thank you!
runtime/src/test_utils.rs
Outdated
| .delegated_addresses_source | ||
| .get(address) | ||
| .and_then(|a| self.resolve_address(a)); | ||
| } |
There was a problem hiding this comment.
This change will disappear of the delegated addresses are mapped in the existing mapping.
anorth
changed the title
…lecoin-project#1118) * feat: Partial implementation of FIP-0048 * Address review
Extracted from
next.This is a partial implementation of FIP-0048. This PR:
Actorobject, that includes an Optional field for Predictable addressesPlaceholderactor that has no state and accepts all callslookup_delegated_addresssyscall to retrieve an actor's f4 addressIt does NOT introduce the new Exec4 method specified in FIP-0048, as that method requires implementation of the Ethereum Address Manager actor. That will be included in a future PR.