Resolve to ID addresses when handling message selection

chain/messagepool/messagepool.go

@@ -126,8 +126,10 @@ type MessagePool struct {
 
 	republished map[cid.Cid]struct{}
 
+	// only pubkey addresses
 	localAddrs map[address.Address]struct{}
 
+	// only pubkey addresses
 	pending map[address.Address]*msgSet
 
 	curTsLk sync.Mutex // DO NOT LOCK INSIDE lk
@@ -443,7 +445,14 @@ func (mp *MessagePool) runLoop() {
 }
 
 func (mp *MessagePool) addLocal(m *types.SignedMessage) error {
-	mp.localAddrs[m.Message.From] = struct{}{}
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), m.Message.From, mp.curTs)
+	if err != nil {
+		log.Debugf("mpooladdlocal failed to resolve sender: %s", err)
+		return err
+	}
+
+	mp.localAddrs[sk] = struct{}{}
 
 	msgb, err := m.Serialize()
 	if err != nil {
@@ -475,7 +484,7 @@ func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.T
 		return false, xerrors.Errorf("message will not be included in a block: %w", err)
 	}
 
-	// this checks if the GasFeeCap is suffisciently high for inclusion in the next 20 blocks
+	// this checks if the GasFeeCap is sufficiently high for inclusion in the next 20 blocks
 	// if the GasFeeCap is too low, we soft reject the message (Ignore in pubsub) and rely
 	// on republish to push it through later, if the baseFee has fallen.
 	// this is a defensive check that stops minimum baseFee spam attacks from overloading validation
@@ -645,7 +654,14 @@ func (mp *MessagePool) checkBalance(m *types.SignedMessage, curTs *types.TipSet)
 	// add Value for soft failure check
 	//requiredFunds = types.BigAdd(requiredFunds, m.Message.Value)
 
-	mset, ok := mp.pending[m.Message.From]
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), m.Message.From, mp.curTs)
+	if err != nil {
+		log.Debugf("mpoolcheckbalance failed to resolve sender: %s", err)
+		return err
+	}
+
+	mset, ok := mp.pending[sk]
 	if ok {
 		requiredFunds = types.BigAdd(requiredFunds, mset.getRequiredFunds(m.Message.Nonce))
 	}
@@ -752,15 +768,22 @@ func (mp *MessagePool) addLocked(m *types.SignedMessage, strict, untrusted bool)
 		return err
 	}
 
-	mset, ok := mp.pending[m.Message.From]
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), m.Message.From, mp.curTs)
+	if err != nil {
+		log.Debugf("mpooladd failed to resolve sender: %s", err)
+		return err
+	}
+
+	mset, ok := mp.pending[sk]
 	if !ok {
-		nonce, err := mp.getStateNonce(m.Message.From, mp.curTs)
+		nonce, err := mp.getStateNonce(sk, mp.curTs)
 		if err != nil {
 			return xerrors.Errorf("failed to get initial actor nonce: %w", err)
 		}
 
 		mset = newMsgSet(nonce)
-		mp.pending[m.Message.From] = mset
+		mp.pending[sk] = mset
 	}
 
 	incr, err := mset.add(m, mp, strict, untrusted)
@@ -811,7 +834,14 @@ func (mp *MessagePool) getNonceLocked(addr address.Address, curTs *types.TipSet)
 		return 0, err
 	}
 
-	mset, ok := mp.pending[addr]
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), addr, mp.curTs)
+	if err != nil {
+		log.Debugf("mpoolgetnonce failed to resolve sender: %s", err)
+		return 0, err
+	}
+
+	mset, ok := mp.pending[sk]
 	if ok {
 		if stateNonce > mset.nextNonce {
 			log.Errorf("state nonce was larger than mset.nextNonce (%d > %d)", stateNonce, mset.nextNonce)
@@ -891,7 +921,14 @@ func (mp *MessagePool) Remove(from address.Address, nonce uint64, applied bool)
 }
 
 func (mp *MessagePool) remove(from address.Address, nonce uint64, applied bool) {
-	mset, ok := mp.pending[from]
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), from, mp.curTs)
+	if err != nil {
+		log.Debugf("mpoolremove failed to resolve sender: %s", err)
+		return
+	}
+
+	mset, ok := mp.pending[sk]
 	if !ok {
 		return
 	}
@@ -949,7 +986,14 @@ func (mp *MessagePool) PendingFor(a address.Address) ([]*types.SignedMessage, *t
 }
 
 func (mp *MessagePool) pendingFor(a address.Address) []*types.SignedMessage {
-	mset := mp.pending[a]
+	// TODO: Is context.TODO() safe here? Idk how Go works.
+	sk, err := mp.api.StateAccountKey(context.TODO(), a, mp.curTs)
+	if err != nil {
+		log.Debugf("mpoolpendingfor failed to resolve sender: %s", err)
+		return nil
+	}
+
+	mset := mp.pending[sk]
 	if mset == nil || len(mset.msgs) == 0 {
 		return nil
 	}
@@ -1303,7 +1347,14 @@ func (mp *MessagePool) loadLocal() error {
 			log.Errorf("adding local message: %+v", err)
 		}
 
-		mp.localAddrs[sm.Message.From] = struct{}{}
+		// TODO: Is context.TODO() safe here? Idk how Go works.
+		sk, err := mp.api.StateAccountKey(context.TODO(), sm.Message.From, mp.curTs)
+		if err != nil {
+			log.Debugf("mpoolloadLocal failed to resolve sender: %s", err)
+			return err
+		}
+
+		mp.localAddrs[sk] = struct{}{}
 	}
 
 	return nil

chain/messagepool/provider.go

@@ -37,6 +37,8 @@ type mpoolProvider struct {
 	ps *pubsub.PubSub
 }
 
+var _ Provider = (*mpoolProvider)(nil)
+
 func NewProvider(sm *stmgr.StateManager, ps *pubsub.PubSub) Provider {
 	return &mpoolProvider{sm: sm, ps: ps}
 }

chain/messagepool/selection.go

@@ -543,10 +543,16 @@ func (mp *MessagePool) selectPriorityMessages(pending map[address.Address]map[ui
 	var chains []*msgChain
 	priority := mpCfg.PriorityAddrs
 	for _, actor := range priority {
-		mset, ok := pending[actor]
+		pk, err := mp.api.StateAccountKey(context.TODO(), actor, mp.curTs)
+		if err != nil {
+			log.Debugf("mpooladdlocal failed to resolve sender: %s", err)
+			return nil, gasLimit
+		}
+
+		mset, ok := pending[pk]
 		if ok {
 			// remove actor from pending set as we are already processed these messages
-			delete(pending, actor)
+			delete(pending, pk)
 			// create chains for the priority actor
 			next := mp.createMessageChains(actor, mset, baseFee, ts)
 			chains = append(chains, next...)

chain/store/store.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/filecoin-project/lotus/chain/state"
+
 	"golang.org/x/sync/errgroup"
 
 	"github.com/filecoin-project/go-state-types/crypto"
@@ -1008,17 +1010,33 @@ type BlockMessages struct {
 func (cs *ChainStore) BlockMsgsForTipset(ts *types.TipSet) ([]BlockMessages, error) {
 	applied := make(map[address.Address]uint64)
 
+	cst := cbor.NewCborStore(cs.stateBlockstore)
+	st, err := state.LoadStateTree(cst, ts.Blocks()[0].ParentStateRoot)
+	if err != nil {
+		return nil, xerrors.Errorf("failed to load state tree")
+	}
+
 	selectMsg := func(m *types.Message) (bool, error) {
-		// The first match for a sender is guaranteed to have correct nonce -- the block isn't valid otherwise
+		var sender address.Address
-		if _, ok := applied[m.From]; !ok {
+		if ts.Height() >= build.UpgradeHyperdriveHeight {
-			applied[m.From] = m.Nonce
+			sender, err = st.LookupID(m.From)
+			if err != nil {
+				return false, err
+			}
+		} else {
+			sender = m.From
 		}
 
-		if applied[m.From] != m.Nonce {
+		// The first match for a sender is guaranteed to have correct nonce -- the block isn't valid otherwise
+		if _, ok := applied[sender]; !ok {
+			applied[sender] = m.Nonce
+		}
+
+		if applied[sender] != m.Nonce {
 			return false, nil
 		}
 
-		applied[m.From]++
+		applied[sender]++
 
 		return true, nil
 	}

chain/sync.go

@@ -1084,9 +1084,19 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
 
 		// Phase 2: (Partial) semantic validation:
 		// the sender exists and is an account actor, and the nonces make sense
-		if _, ok := nonces[m.From]; !ok {
+		var sender address.Address
+		if syncer.sm.GetNtwkVersion(ctx, b.Header.Height) >= network.Version13 {
+			sender, err = st.LookupID(m.From)
+			if err != nil {
+				return err
+			}
+		} else {
+			sender = m.From
+		}
+
+		if _, ok := nonces[sender]; !ok {
 			// `GetActor` does not validate that this is an account actor.
-			act, err := st.GetActor(m.From)
+			act, err := st.GetActor(sender)
 			if err != nil {
 				return xerrors.Errorf("failed to get actor: %w", err)
 			}
@@ -1094,13 +1104,13 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
 			if !builtin.IsAccountActor(act.Code) {
 				return xerrors.New("Sender must be an account actor")
 			}
-			nonces[m.From] = act.Nonce
+			nonces[sender] = act.Nonce
 		}
 
-		if nonces[m.From] != m.Nonce {
+		if nonces[sender] != m.Nonce {
-			return xerrors.Errorf("wrong nonce (exp: %d, got: %d)", nonces[m.From], m.Nonce)
+			return xerrors.Errorf("wrong nonce (exp: %d, got: %d)", nonces[sender], m.Nonce)
 		}
-		nonces[m.From]++
+		nonces[sender]++
 
 		return nil
 	}

chain/sync_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/filecoin-project/go-state-types/crypto"
 
 	"github.com/filecoin-project/go-state-types/network"
-
 	"github.com/filecoin-project/lotus/chain/stmgr"
 
 	"github.com/ipfs/go-cid"
@@ -108,6 +107,7 @@ func prepSyncTest(t testing.TB, h int) *syncTestUtil {
 	}
 
 	tu.addSourceNode(stmgr.DefaultUpgradeSchedule(), h)
+
 	//tu.checkHeight("source", source, h)
 
 	// separate logs
@@ -730,7 +730,6 @@ func TestBadNonce(t *testing.T) {
 
 	// Produce a message from the banker with a bad nonce
 	makeBadMsg := func() *types.SignedMessage {
-
 		msg := types.Message{
 			To:   tu.g.Banker(),
 			From: tu.g.Banker(),
@@ -761,6 +760,114 @@ func TestBadNonce(t *testing.T) {
 	tu.mineOnBlock(base, 0, []int{0}, true, true, msgs, 0)
 }
 
+// This test introduces a block that has 2 messages, with the same sender, and same nonce.
+// One of the messages uses the sender's robust address, the other uses the ID address.
+// Such a block is invalid and should not sync.
+func TestMismatchedNoncesRobustID(t *testing.T) {
+	v5h := abi.ChainEpoch(4)
+	tu := prepSyncTestWithV5Height(t, int(v5h+5), v5h)
+
+	base := tu.g.CurTipset
+
+	// Get the banker from computed tipset state, not the parent.
+	st, _, err := tu.g.StateManager().TipSetState(context.TODO(), base.TipSet())
+	require.NoError(t, err)
+	ba, err := tu.g.StateManager().LoadActorRaw(context.TODO(), tu.g.Banker(), st)
+	require.NoError(t, err)
+
+	// Produce a message from the banker
+	makeMsg := func(id bool) *types.SignedMessage {
+		sender := tu.g.Banker()
+		if id {
+			s, err := tu.nds[0].StateLookupID(context.TODO(), sender, base.TipSet().Key())
+			require.NoError(t, err)
+			sender = s
+		}
+
+		msg := types.Message{
+			To:   tu.g.Banker(),
+			From: sender,
+
+			Nonce: ba.Nonce,
+
+			Value: types.NewInt(1),
+
+			Method: 0,
+
+			GasLimit:   100_000_000,
+			GasFeeCap:  types.NewInt(0),
+			GasPremium: types.NewInt(0),
+		}
+
+		sig, err := tu.g.Wallet().WalletSign(context.TODO(), tu.g.Banker(), msg.Cid().Bytes(), api.MsgMeta{})
+		require.NoError(t, err)
+
+		return &types.SignedMessage{
+			Message:   msg,
+			Signature: *sig,
+		}
+	}
+
+	msgs := make([][]*types.SignedMessage, 1)
+	msgs[0] = []*types.SignedMessage{makeMsg(false), makeMsg(true)}
+
+	tu.mineOnBlock(base, 0, []int{0}, true, true, msgs, 0)
+}
+
+// This test introduces a block that has 2 messages, with the same sender, and nonces N and N+1 (so both can be included in a block)
+// One of the messages uses the sender's robust address, the other uses the ID address.
+// Such a block is valid and should sync.
+func TestMatchedNoncesRobustID(t *testing.T) {
+	v5h := abi.ChainEpoch(4)
+	tu := prepSyncTestWithV5Height(t, int(v5h+5), v5h)
+
+	base := tu.g.CurTipset
+
+	// Get the banker from computed tipset state, not the parent.
+	st, _, err := tu.g.StateManager().TipSetState(context.TODO(), base.TipSet())
+	require.NoError(t, err)
+	ba, err := tu.g.StateManager().LoadActorRaw(context.TODO(), tu.g.Banker(), st)
+	require.NoError(t, err)
+
+	// Produce a message from the banker with specified nonce
+	makeMsg := func(n uint64, id bool) *types.SignedMessage {
+		sender := tu.g.Banker()
+		if id {
+			s, err := tu.nds[0].StateLookupID(context.TODO(), sender, base.TipSet().Key())
+			require.NoError(t, err)
+			sender = s
+		}
+
+		msg := types.Message{
+			To:   tu.g.Banker(),
+			From: sender,
+
+			Nonce: n,
+
+			Value: types.NewInt(1),
+
+			Method: 0,
+
+			GasLimit:   100_000_000,
+			GasFeeCap:  types.NewInt(0),
+			GasPremium: types.NewInt(0),
+		}
+
+		sig, err := tu.g.Wallet().WalletSign(context.TODO(), tu.g.Banker(), msg.Cid().Bytes(), api.MsgMeta{})
+		require.NoError(t, err)
+
+		return &types.SignedMessage{
+			Message:   msg,
+			Signature: *sig,
+		}
+	}
+
+	msgs := make([][]*types.SignedMessage, 1)
+	msgs[0] = []*types.SignedMessage{makeMsg(ba.Nonce, false), makeMsg(ba.Nonce+1, true)}
+
+	tu.mineOnBlock(base, 0, []int{0}, true, false, msgs, 0)
+}
+
 func BenchmarkSyncBasic(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		runSyncBenchLength(b, 100)

documentation/misc/actors_version_checklist.md

@@ -12,6 +12,6 @@
 - [ ] Update `chain/stmgr/forks.go`
   - [ ] Schedule
   - [ ] Migration
-- [ ] Update upgrade schedule in `api/test/test.go`
+- [ ] Update upgrade schedule in `api/test/test.go` and `chain/sync_test.go`
 - [ ] Update `NewestNetworkVersion` in `build/params_shared_vals.go`
 - [ ] Register in init in `chain/stmgr/utils.go`