diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go index c2566ae24..299634c6f 100644 --- a/chain/messagepool/messagepool.go +++ b/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 diff --git a/chain/messagepool/provider.go b/chain/messagepool/provider.go index 5a6c751bc..75a2efe91 100644 --- a/chain/messagepool/provider.go +++ b/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} } diff --git a/chain/messagepool/selection.go b/chain/messagepool/selection.go index 6c9d506ef..af450645f 100644 --- a/chain/messagepool/selection.go +++ b/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...) diff --git a/chain/store/store.go b/chain/store/store.go index dfde93fc7..5414b12fe 100644 --- a/chain/store/store.go +++ b/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 - if _, ok := applied[m.From]; !ok { - applied[m.From] = m.Nonce + var sender address.Address + if ts.Height() >= build.UpgradeHyperdriveHeight { + 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 } diff --git a/chain/sync.go b/chain/sync.go index 66c9c18bd..d908f3fd8 100644 --- a/chain/sync.go +++ b/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 { - return xerrors.Errorf("wrong nonce (exp: %d, got: %d)", nonces[m.From], m.Nonce) + if nonces[sender] != m.Nonce { + return xerrors.Errorf("wrong nonce (exp: %d, got: %d)", nonces[sender], m.Nonce) } - nonces[m.From]++ + nonces[sender]++ return nil } diff --git a/chain/sync_test.go b/chain/sync_test.go index 095b224ad..4aa3f6fdf 100644 --- a/chain/sync_test.go +++ b/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) diff --git a/documentation/misc/actors_version_checklist.md b/documentation/misc/actors_version_checklist.md index 7931acbcc..769e9da42 100644 --- a/documentation/misc/actors_version_checklist.md +++ b/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`