Add tests for PCB/PCA batch splitting

node/impl/full/gas.go

@@ -384,13 +384,11 @@ func gasEstimateGasLimit(
 func (m *GasModule) GasEstimateMessageGas(ctx context.Context, msg *types.Message, spec *api.MessageSendSpec, _ types.TipSetKey) (*types.Message, error) {
 	if msg.GasLimit == 0 {
 		gasLimit, err := m.GasEstimateGasLimit(ctx, msg, types.EmptyTSK)
-		log.Errorf("GasEstimateMessageGas GasLimit: %f", gasLimit)
 		if err != nil {
 			return nil, err
 		}
 		msg.GasLimit = int64(float64(gasLimit) * m.Mpool.GetConfig().GasLimitOverestimation)
 
-		log.Errorf("GasEstimateMessageGas GasLimit: %f, GasLimitWithOverestimation", gasLimit, msg.GasLimit)
 		// Gas overestimation can cause us to exceed the block gas limit, cap it.
 		if msg.GasLimit > build.BlockGasLimit {
 			msg.GasLimit = build.BlockGasLimit

storage/pipeline/commit_batch.go

@@ -290,11 +290,6 @@ func (b *CommitBatcher) processBatch(cfg sealiface.Config, sectors []abi.SectorN
 	collateral := big.Zero()
 
 	for _, sector := range sectors {
-		if len(infos) >= cfg.MaxCommitBatch {
-			log.Infow("commit batch full")
-			break
-		}
-
 		res.Sectors = append(res.Sectors, sector)
 
 		sc, err := b.getSectorCollateral(sector, ts.Key())

storage/pipeline/commit_batch_test.go

@@ -201,8 +201,7 @@ func TestCommitBatcher(t *testing.T) {
 
 			if batch {
 				s.EXPECT().StateNetworkVersion(gomock.Any(), gomock.Any()).Return(network.Version13, nil)
-				s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Message{GasLimit: 1000000}, nil)
+				s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Message{GasLimit: 100000}, nil)
-				//s.EXPECT().ChainBaseFee(gomock.Any(), gomock.Any()).Return(basefee, nil)
 			}
 
 			s.EXPECT().MpoolPushMessage(gomock.Any(), funMatcher(func(i interface{}) bool {
@@ -225,6 +224,48 @@ func TestCommitBatcher(t *testing.T) {
 		}
 	}
 
+	expectProcessBatch := func(expect []abi.SectorNumber, aboveBalancer, failOnePCI bool, gasOverLimit bool) action {
+		return func(t *testing.T, s *mocks.MockCommitBatcherApi, pcb *pipeline.CommitBatcher) promise {
+			s.EXPECT().StateMinerInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(api.MinerInfo{Owner: t0123, Worker: t0123}, nil)
+
+			ti := len(expect)
+			batch := false
+			if ti >= minBatch {
+				batch = true
+				ti = 1
+			}
+
+			if !aboveBalancer {
+				batch = false
+				ti = len(expect)
+			}
+
+			pciC := len(expect)
+			if failOnePCI {
+				s.EXPECT().StateSectorPreCommitInfo(gomock.Any(), gomock.Any(), abi.SectorNumber(1), gomock.Any()).Return(nil, nil).Times(1) // not found
+				pciC = len(expect) - 1
+				if !batch {
+					ti--
+				}
+			}
+			s.EXPECT().StateSectorPreCommitInfo(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&minertypes.SectorPreCommitOnChainInfo{
+				PreCommitDeposit: big.Zero(),
+			}, nil).Times(pciC)
+			s.EXPECT().StateMinerInitialPledgeCollateral(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(big.Zero(), nil).Times(pciC)
+
+			if batch {
+				s.EXPECT().StateNetworkVersion(gomock.Any(), gomock.Any()).Return(network.Version18, nil)
+				if gasOverLimit {
+					s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, &api.ErrOutOfGas{})
+				} else {
+					s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Message{GasLimit: 100000}, nil)
+				}
+
+			}
+			return nil
+		}
+	}
+
 	flush := func(expect []abi.SectorNumber, aboveBalancer, failOnePCI bool) action {
 		return func(t *testing.T, s *mocks.MockCommitBatcherApi, pcb *pipeline.CommitBatcher) promise {
 			_ = expectSend(expect, aboveBalancer, failOnePCI)(t, s, pcb)
@@ -310,6 +351,14 @@ func TestCommitBatcher(t *testing.T) {
 				addSectors(getSectors(maxBatch), true),
 			},
 		},
+		"addMax-aboveBalancer-gasAboveLimit": {
+			actions: []action{
+				expectProcessBatch(getSectors(maxBatch), true, false, true),
+				expectSend(getSectors(maxBatch)[:maxBatch/2], true, false),
+				expectSend(getSectors(maxBatch)[maxBatch/2:], true, false),
+				addSectors(getSectors(maxBatch), true),
+			},
+		},
 		"addSingle-belowBalancer": {
 			actions: []action{
 				addSector(0, false),

storage/pipeline/precommit_batch_test.go

@@ -156,22 +156,34 @@ func TestPrecommitBatcher(t *testing.T) {
 	}
 
 	//stm: @CHAIN_STATE_MINER_INFO_001, @CHAIN_STATE_NETWORK_VERSION_001
-	expectSend := func(expect []abi.SectorNumber) action {
+	expectSend := func(expect []abi.SectorNumber, gasOverLimit bool) action {
+		return func(t *testing.T, s *mocks.MockPreCommitBatcherApi, pcb *pipeline.PreCommitBatcher) promise {
+			s.EXPECT().StateMinerInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(api.MinerInfo{Owner: t0123, Worker: t0123}, nil)
+			if gasOverLimit {
+				s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, &api.ErrOutOfGas{})
+			} else {
+				s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Message{GasLimit: 100000}, nil)
+			}
+
+			if !gasOverLimit {
+				s.EXPECT().MpoolPushMessage(gomock.Any(), funMatcher(func(i interface{}) bool {
+					b := i.(*types.Message)
+					var params miner6.PreCommitSectorBatchParams
+					require.NoError(t, params.UnmarshalCBOR(bytes.NewReader(b.Params)))
+					for s, number := range expect {
+						require.Equal(t, number, params.Sectors[s].SectorNumber)
+					}
+					return true
+				}), gomock.Any()).Return(dummySmsg, nil)
+			}
+			return nil
+		}
+	}
+
+	expectInitialCalls := func() action {
 		return func(t *testing.T, s *mocks.MockPreCommitBatcherApi, pcb *pipeline.PreCommitBatcher) promise {
 			s.EXPECT().ChainHead(gomock.Any()).Return(makeBFTs(t, big.NewInt(10001), 1), nil)
 			s.EXPECT().StateNetworkVersion(gomock.Any(), gomock.Any()).Return(network.Version14, nil)
-
-			s.EXPECT().StateMinerInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(api.MinerInfo{Owner: t0123, Worker: t0123}, nil)
-			s.EXPECT().GasEstimateMessageGas(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Message{GasLimit: 100000}, nil)
-			s.EXPECT().MpoolPushMessage(gomock.Any(), funMatcher(func(i interface{}) bool {
-				b := i.(*types.Message)
-				var params miner6.PreCommitSectorBatchParams
-				require.NoError(t, params.UnmarshalCBOR(bytes.NewReader(b.Params)))
-				for s, number := range expect {
-					require.Equal(t, number, params.Sectors[s].SectorNumber)
-				}
-				return true
-			}), gomock.Any()).Return(dummySmsg, nil)
 			return nil
 		}
 	}
@@ -199,7 +211,8 @@ func TestPrecommitBatcher(t *testing.T) {
 
 	flush := func(expect []abi.SectorNumber) action {
 		return func(t *testing.T, s *mocks.MockPreCommitBatcherApi, pcb *pipeline.PreCommitBatcher) promise {
-			_ = expectSend(expect)(t, s, pcb)
+			_ = expectInitialCalls()(t, s, pcb)
+			_ = expectSend(expect, false)(t, s, pcb)
 
 			r, err := pcb.Flush(ctx)
 			require.NoError(t, err)
@@ -241,7 +254,17 @@ func TestPrecommitBatcher(t *testing.T) {
 		},
 		"addMax": {
 			actions: []action{
-				expectSend(getSectors(maxBatch)),
+				expectInitialCalls(),
+				expectSend(getSectors(maxBatch), false),
+				addSectors(getSectors(maxBatch), true),
+			},
+		},
+		"addMax-gasAboveLimit": {
+			actions: []action{
+				expectInitialCalls(),
+				expectSend(getSectors(maxBatch), true),
+				expectSend(getSectors(maxBatch)[:maxBatch/2], false),
+				expectSend(getSectors(maxBatch)[maxBatch/2:], false),
 				addSectors(getSectors(maxBatch), true),
 			},
 		},