Refactor LookupID* APIs in StateManager and StateTree

The naming of `LookupID` can cause confusion when resolving actor IDs vs ID addresses. To avoid this: * Refactor `StateTree` `LookupID` to `LookupIDAddress`, because it returns ID address. * Refactor `StateManager` `LookupID` to `LookupIDAddress` because it also returns ID address via a chain call to `StateTree`. * Introduce a new API `StateManager` dedicated to resolving address to actor ID, called `LookupID` which returns `abi.ActorID`. For context, see: * #11723 (comment)
filecoin-project · Apr 24, 2024 · f907354 · f907354
1 parent d23ea76
commit f907354
Show file tree

Hide file tree

Showing 12 changed files with 43 additions and 26 deletions.
diff --git a/chain/consensus/common.go b/chain/consensus/common.go
@@ -220,7 +220,7 @@ func checkBlockMessages(ctx context.Context, sm *stmgr.StateManager, cs *store.C
 		// the sender exists and is an account actor, and the nonces make sense
 		var sender address.Address
 		if nv >= network.Version13 {
-			sender, err = st.LookupID(m.From)
+			sender, err = st.LookupIDAddress(m.From)
 			if err != nil {
 				return xerrors.Errorf("failed to lookup sender %s: %w", m.From, err)
 			}

diff --git a/chain/state/statetree.go b/chain/state/statetree.go
@@ -230,7 +230,7 @@ func NewStateTree(cst cbor.IpldStore, ver types.StateTreeVersion) (*StateTree, e
 		Store:   cst,
 		snaps:   newStateSnaps(),
 	}
-	s.lookupIDFun = s.lookupIDinternal
+	s.lookupIDFun = s.lookupInternalIDAddress
 	return s, nil
 }
 
@@ -302,13 +302,13 @@ func LoadStateTree(cst cbor.IpldStore, c cid.Cid) (*StateTree, error) {
 		Store:   cst,
 		snaps:   newStateSnaps(),
 	}
-	s.lookupIDFun = s.lookupIDinternal
+	s.lookupIDFun = s.lookupInternalIDAddress
 
 	return s, nil
 }
 
 func (st *StateTree) SetActor(addr address.Address, act *types.Actor) error {
-	iaddr, err := st.LookupID(addr)
+	iaddr, err := st.LookupIDAddress(addr)
 	if err != nil {
 		return xerrors.Errorf("ID lookup failed: %w", err)
 	}
@@ -318,7 +318,7 @@ func (st *StateTree) SetActor(addr address.Address, act *types.Actor) error {
 	return nil
 }
 
-func (st *StateTree) lookupIDinternal(addr address.Address) (address.Address, error) {
+func (st *StateTree) lookupInternalIDAddress(addr address.Address) (address.Address, error) {
 	act, err := st.GetActor(init_.Address)
 	if err != nil {
 		return address.Undef, xerrors.Errorf("getting init actor: %w", err)
@@ -339,8 +339,8 @@ func (st *StateTree) lookupIDinternal(addr address.Address) (address.Address, er
 	return a, err
 }
 
-// LookupID gets the ID address of this actor's `addr` stored in the `InitActor`.
-func (st *StateTree) LookupID(addr address.Address) (address.Address, error) {
+// LookupIDAddress gets the ID address of this actor's `addr` stored in the `InitActor`.
+func (st *StateTree) LookupIDAddress(addr address.Address) (address.Address, error) {
 	if addr.Protocol() == address.ID {
 		return addr, nil
 	}
@@ -366,7 +366,7 @@ func (st *StateTree) GetActor(addr address.Address) (*types.Actor, error) {
 	}
 
 	// Transform `addr` to its ID format.
-	iaddr, err := st.LookupID(addr)
+	iaddr, err := st.LookupIDAddress(addr)
 	if err != nil {
 		if xerrors.Is(err, types.ErrActorNotFound) {
 			return nil, xerrors.Errorf("resolution lookup failed (%s): %w", addr, err)
@@ -411,7 +411,7 @@ func (st *StateTree) DeleteActor(addr address.Address) error {
 		return xerrors.Errorf("DeleteActor called on undefined address")
 	}
 
-	iaddr, err := st.LookupID(addr)
+	iaddr, err := st.LookupIDAddress(addr)
 	if err != nil {
 		if xerrors.Is(err, types.ErrActorNotFound) {
 			return xerrors.Errorf("resolution lookup failed (%s): %w", addr, err)

diff --git a/chain/stmgr/actors.go b/chain/stmgr/actors.go
@@ -542,7 +542,7 @@ func (sm *StateManager) MarketBalance(ctx context.Context, addr address.Address,
 		return api.MarketBalance{}, err
 	}
 
-	addr, err = sm.LookupID(ctx, addr, ts)
+	addr, err = sm.LookupIDAddress(ctx, addr, ts)
 	if err != nil {
 		return api.MarketBalance{}, err
 	}

diff --git a/chain/stmgr/rpc/rpcstatemanager.go b/chain/stmgr/rpc/rpcstatemanager.go
@@ -44,7 +44,7 @@ func (s *RPCStateManager) LoadActorTsk(ctx context.Context, addr address.Address
 	return s.gapi.StateGetActor(ctx, addr, tsk)
 }
 
-func (s *RPCStateManager) LookupID(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error) {
+func (s *RPCStateManager) LookupIDAddress(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error) {
 	return s.gapi.StateLookupID(ctx, addr, ts.Key())
 }
 

diff --git a/chain/stmgr/searchwait.go b/chain/stmgr/searchwait.go
@@ -243,7 +243,7 @@ func (sm *StateManager) searchBackForMsg(ctx context.Context, from *types.TipSet
 		return nil, nil, cid.Undef, xerrors.Errorf("failed to load initital tipset")
 	}
 
-	mFromId, err := sm.LookupID(ctx, m.VMMessage().From, from)
+	mFromId, err := sm.LookupIDAddress(ctx, m.VMMessage().From, from)
 	if err != nil {
 		return nil, nil, cid.Undef, xerrors.Errorf("looking up From id address: %w", err)
 	}

diff --git a/chain/stmgr/stmgr.go b/chain/stmgr/stmgr.go
@@ -49,7 +49,7 @@ type StateManagerAPI interface {
 	Call(ctx context.Context, msg *types.Message, ts *types.TipSet) (*api.InvocResult, error)
 	GetPaychState(ctx context.Context, addr address.Address, ts *types.TipSet) (*types.Actor, paych.State, error)
 	LoadActorTsk(ctx context.Context, addr address.Address, tsk types.TipSetKey) (*types.Actor, error)
-	LookupID(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)
+	LookupIDAddress(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)
 	ResolveToDeterministicAddress(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)
 }
 
@@ -400,13 +400,30 @@ func (sm *StateManager) GetBlsPublicKey(ctx context.Context, addr address.Addres
 	return kaddr.Payload(), nil
 }
 
-func (sm *StateManager) LookupID(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error) {
+func (sm *StateManager) LookupIDAddress(_ context.Context, addr address.Address, ts *types.TipSet) (address.Address, error) {
+	// Check for the fast route first to avoid unnecessary CBOR store instantiation and state tree load.
+	if addr.Protocol() == address.ID {
+		return addr, nil
+	}
+
 	cst := cbor.NewCborStore(sm.cs.StateBlockstore())
 	state, err := state.LoadStateTree(cst, sm.parentState(ts))
 	if err != nil {
 		return address.Undef, xerrors.Errorf("load state tree: %w", err)
 	}
-	return state.LookupID(addr)
+	return state.LookupIDAddress(addr)
+}
+
+func (sm *StateManager) LookupID(ctx context.Context, addr address.Address, ts *types.TipSet) (abi.ActorID, error) {
+	idAddr, err := sm.LookupIDAddress(ctx, addr, ts)
+	if err != nil {
+		return 0, xerrors.Errorf("state manager lookup id: %w", err)
+	}
+	id, err := address.IDFromAddress(idAddr)
+	if err != nil {
+		return 0, xerrors.Errorf("resolve actor id: id from addr: %w", err)
+	}
+	return abi.ActorID(id), nil
 }
 
 func (sm *StateManager) LookupRobustAddress(ctx context.Context, idAddr address.Address, ts *types.TipSet) (address.Address, error) {

diff --git a/chain/store/messages.go b/chain/store/messages.go
@@ -119,7 +119,7 @@ func (cs *ChainStore) BlockMsgsForTipset(ctx context.Context, ts *types.TipSet)
 		var sender address.Address
 		if ts.Height() >= build.UpgradeHyperdriveHeight {
 			if useIds {
-				sender, err = st.LookupID(m.From)
+				sender, err = st.LookupIDAddress(m.From)
 				if err != nil {
 					return false, xerrors.Errorf("failed to resolve sender: %w", err)
 				}
@@ -131,14 +131,14 @@ func (cs *ChainStore) BlockMsgsForTipset(ctx context.Context, ts *types.TipSet)
 					// uh-oh, we actually have an ID-sender!
 					useIds = true
 					for robust, nonce := range applied {
-						resolved, err := st.LookupID(robust)
+						resolved, err := st.LookupIDAddress(robust)
 						if err != nil {
 							return false, xerrors.Errorf("failed to resolve sender: %w", err)
 						}
 						applied[resolved] = nonce
 					}
 
-					sender, err = st.LookupID(m.From)
+					sender, err = st.LookupIDAddress(m.From)
 					if err != nil {
 						return false, xerrors.Errorf("failed to resolve sender: %w", err)
 					}

diff --git a/chain/vm/runtime.go b/chain/vm/runtime.go
@@ -111,7 +111,7 @@ func (rt *Runtime) TotalFilCircSupply() abi.TokenAmount {
 }
 
 func (rt *Runtime) ResolveAddress(addr address.Address) (ret address.Address, ok bool) {
-	r, err := rt.state.LookupID(addr)
+	r, err := rt.state.LookupIDAddress(addr)
 	if err != nil {
 		if xerrors.Is(err, types.ErrActorNotFound) {
 			return address.Undef, false

diff --git a/chain/vm/vm.go b/chain/vm/vm.go
@@ -902,7 +902,7 @@ func (vm *LegacyVM) transfer(from, to address.Address, amt types.BigInt, network
 			return aerrors.Newf(exitcode.SysErrForbidden, "attempted to transfer negative value: %s", amt)
 		}
 
-		fromID, err = vm.cstate.LookupID(from)
+		fromID, err = vm.cstate.LookupIDAddress(from)
 		if err != nil {
 			return aerrors.Fatalf("transfer failed when resolving sender address: %s", err)
 		}
@@ -921,7 +921,7 @@ func (vm *LegacyVM) transfer(from, to address.Address, amt types.BigInt, network
 			return nil
 		}
 
-		toID, err = vm.cstate.LookupID(to)
+		toID, err = vm.cstate.LookupIDAddress(to)
 		if err != nil {
 			return aerrors.Fatalf("transfer failed when resolving receiver address: %s", err)
 		}
@@ -935,12 +935,12 @@ func (vm *LegacyVM) transfer(from, to address.Address, amt types.BigInt, network
 			return nil
 		}
 
-		fromID, err = vm.cstate.LookupID(from)
+		fromID, err = vm.cstate.LookupIDAddress(from)
 		if err != nil {
 			return aerrors.Fatalf("transfer failed when resolving sender address: %s", err)
 		}
 
-		toID, err = vm.cstate.LookupID(to)
+		toID, err = vm.cstate.LookupIDAddress(to)
 		if err != nil {
 			return aerrors.Fatalf("transfer failed when resolving receiver address: %s", err)
 		}

diff --git a/itests/migration_test.go b/itests/migration_test.go
@@ -584,7 +584,7 @@ func TestMigrationNV18(t *testing.T) {
 	// check the EthZeroAddress
 	ethZeroAddr, err := (ethtypes.EthAddress{}).ToFilecoinAddress()
 	require.NoError(t, err)
-	ethZeroAddrID, err := newStateTree.LookupID(ethZeroAddr)
+	ethZeroAddrID, err := newStateTree.LookupIDAddress(ethZeroAddr)
 	require.NoError(t, err)
 	ethZeroActor, err := newStateTree.GetActor(ethZeroAddrID)
 	require.NoError(t, err)

diff --git a/node/impl/full/eth_utils.go b/node/impl/full/eth_utils.go
@@ -392,7 +392,7 @@ func lookupEthAddress(addr address.Address, st *state.StateTree) (ethtypes.EthAd
 	}
 
 	// Otherwise, resolve the ID addr.
-	idAddr, err := st.LookupID(addr)
+	idAddr, err := st.LookupIDAddress(addr)
 	if err != nil {
 		return ethtypes.EthAddress{}, err
 	}

diff --git a/node/impl/full/state.go b/node/impl/full/state.go
@@ -486,7 +486,7 @@ func (m *StateModule) StateLookupID(ctx context.Context, addr address.Address, t
 		return address.Undef, xerrors.Errorf("loading tipset %s: %w", tsk, err)
 	}
 
-	ret, err := m.StateManager.LookupID(ctx, addr, ts)
+	ret, err := m.StateManager.LookupIDAddress(ctx, addr, ts)
 	if err != nil && xerrors.Is(err, types.ErrActorNotFound) {
 		return address.Undef, &api.ErrActorNotFound{}
 	}