From f907354300baac92c30857b61b918d598365561c Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Tue, 23 Apr 2024 12:34:54 +0100 Subject: [PATCH] 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: * https://github.com/filecoin-project/lotus/pull/11723#discussion_r1534728607 --- chain/consensus/common.go | 2 +- chain/state/statetree.go | 16 ++++++++-------- chain/stmgr/actors.go | 2 +- chain/stmgr/rpc/rpcstatemanager.go | 2 +- chain/stmgr/searchwait.go | 2 +- chain/stmgr/stmgr.go | 23 ++++++++++++++++++++--- chain/store/messages.go | 6 +++--- chain/vm/runtime.go | 2 +- chain/vm/vm.go | 8 ++++---- itests/migration_test.go | 2 +- node/impl/full/eth_utils.go | 2 +- node/impl/full/state.go | 2 +- 12 files changed, 43 insertions(+), 26 deletions(-) diff --git a/chain/consensus/common.go b/chain/consensus/common.go index a7e5c40d2..8fee0d4c2 100644 --- 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 index 1a6497d04..03cd98d95 100644 --- 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 index f1d615e8d..0dce2c3cd 100644 --- 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 index 9186501ea..fa311e1d4 100644 --- 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 index 356ace23c..3377389b9 100644 --- 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 index cd35d6d19..d88d7dfd1 100644 --- 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 index c23f900d7..4129a9199 100644 --- 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 index 355fcea2b..9ca4f55d0 100644 --- 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 index ba404ab1f..1e0591b6c 100644 --- 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 index 68991a579..e19aaf45f 100644 --- 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 index 50e76f84e..1d7bfac5a 100644 --- 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 index dda889832..ed6fd6685 100644 --- 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{} }