Skip to content

Commit 23c5625

Browse files
authored
Remove RequestBuildBlock on P-Chain Mempool (#3705)
Signed-off-by: Joshua Kim <[email protected]>
1 parent 492298a commit 23c5625

File tree

14 files changed

+112
-91
lines changed

14 files changed

+112
-91
lines changed

vms/platformvm/block/builder/builder.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/ava-labs/avalanchego/ids"
1717
"github.com/ava-labs/avalanchego/snow"
1818
"github.com/ava-labs/avalanchego/snow/consensus/snowman"
19+
"github.com/ava-labs/avalanchego/snow/engine/common"
1920
"github.com/ava-labs/avalanchego/utils/set"
2021
"github.com/ava-labs/avalanchego/utils/timer/mockable"
2122
"github.com/ava-labs/avalanchego/utils/units"
@@ -25,7 +26,7 @@ import (
2526
"github.com/ava-labs/avalanchego/vms/platformvm/status"
2627
"github.com/ava-labs/avalanchego/vms/platformvm/txs"
2728
"github.com/ava-labs/avalanchego/vms/platformvm/txs/fee"
28-
"github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool"
29+
"github.com/ava-labs/avalanchego/vms/txs/mempool"
2930

3031
smblock "github.com/ava-labs/avalanchego/snow/engine/snowman/block"
3132
blockexecutor "github.com/ava-labs/avalanchego/vms/platformvm/block/executor"
@@ -53,7 +54,7 @@ var (
5354

5455
type Builder interface {
5556
smblock.BuildBlockWithContextChainVM
56-
mempool.Mempool
57+
mempool.Mempool[*txs.Tx]
5758

5859
// StartBlockTimer starts to issue block creation requests to advance the
5960
// chain timestamp.
@@ -82,8 +83,9 @@ type Builder interface {
8283

8384
// builder implements a simple builder to convert txs into valid blocks
8485
type builder struct {
85-
mempool.Mempool
86+
mempool.Mempool[*txs.Tx]
8687

88+
toEngine chan<- common.Message
8789
txExecutorBackend *txexecutor.Backend
8890
blkManager blockexecutor.Manager
8991

@@ -95,12 +97,14 @@ type builder struct {
9597
}
9698

9799
func New(
98-
mempool mempool.Mempool,
100+
mempool mempool.Mempool[*txs.Tx],
101+
toEngine chan<- common.Message,
99102
txExecutorBackend *txexecutor.Backend,
100103
blkManager blockexecutor.Manager,
101104
) Builder {
102105
return &builder{
103106
Mempool: mempool,
107+
toEngine: toEngine,
104108
txExecutorBackend: txExecutorBackend,
105109
blkManager: blkManager,
106110
resetTimer: make(chan struct{}, 1),
@@ -143,7 +147,10 @@ func (b *builder) StartBlockTimer() {
143147
}
144148

145149
// Block needs to be issued to advance time.
146-
b.Mempool.RequestBuildBlock(true /*=emptyBlockPermitted*/)
150+
select {
151+
case b.toEngine <- common.PendingTxs:
152+
default:
153+
}
147154

148155
// Invariant: ResetBlockTimer is guaranteed to be called after
149156
// [durationToSleep] returns a value <= 0. This is because we
@@ -225,7 +232,16 @@ func (b *builder) BuildBlockWithContext(
225232
) (snowman.Block, error) {
226233
// If there are still transactions in the mempool, then we need to
227234
// re-trigger block building.
228-
defer b.Mempool.RequestBuildBlock(false /*=emptyBlockPermitted*/)
235+
defer func() {
236+
if b.Mempool.Len() == 0 {
237+
return
238+
}
239+
240+
select {
241+
case b.toEngine <- common.PendingTxs:
242+
default:
243+
}
244+
}()
229245

230246
b.txExecutorBackend.Ctx.Log.Debug("starting to attempt to build a block")
231247

@@ -402,7 +418,7 @@ func packDurangoBlockTxs(
402418
ctx context.Context,
403419
parentID ids.ID,
404420
parentState state.Chain,
405-
mempool mempool.Mempool,
421+
mempool mempool.Mempool[*txs.Tx],
406422
backend *txexecutor.Backend,
407423
manager blockexecutor.Manager,
408424
timestamp time.Time,
@@ -463,7 +479,7 @@ func packEtnaBlockTxs(
463479
ctx context.Context,
464480
parentID ids.ID,
465481
parentState state.Chain,
466-
mempool mempool.Mempool,
482+
mempool mempool.Mempool[*txs.Tx],
467483
backend *txexecutor.Backend,
468484
manager blockexecutor.Manager,
469485
timestamp time.Time,
@@ -563,7 +579,7 @@ func executeTx(
563579
ctx context.Context,
564580
parentID ids.ID,
565581
stateDiff state.Diff,
566-
mempool mempool.Mempool,
582+
mempool mempool.Mempool[*txs.Tx],
567583
backend *txexecutor.Backend,
568584
manager blockexecutor.Manager,
569585
pChainHeight uint64,

vms/platformvm/block/builder/helpers_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151

5252
blockexecutor "github.com/ava-labs/avalanchego/vms/platformvm/block/executor"
5353
txexecutor "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor"
54+
txmempool "github.com/ava-labs/avalanchego/vms/txs/mempool"
5455
)
5556

5657
const (
@@ -67,7 +68,7 @@ type mutableSharedMemory struct {
6768
type environment struct {
6869
Builder
6970
blkManager blockexecutor.Manager
70-
mempool mempool.Mempool
71+
mempool txmempool.Mempool[*txs.Tx]
7172
network *network.Network
7273
sender *enginetest.Sender
7374

@@ -142,11 +143,12 @@ func newEnvironment(t *testing.T, f upgradetest.Fork) *environment { //nolint:un
142143
metrics, err := metrics.New(registerer)
143144
require.NoError(err)
144145

145-
res.mempool, err = mempool.New("mempool", registerer, nil)
146+
res.mempool, err = mempool.New("mempool", registerer)
146147
require.NoError(err)
147148

148149
res.blkManager = blockexecutor.NewManager(
149150
res.mempool,
151+
nil,
150152
metrics,
151153
res.state,
152154
&res.backend,
@@ -161,6 +163,7 @@ func newEnvironment(t *testing.T, f upgradetest.Fork) *environment { //nolint:un
161163
res.backend.Ctx.ValidatorState,
162164
txVerifier,
163165
res.mempool,
166+
nil,
164167
res.backend.Config.PartialSyncPrimaryNetwork,
165168
res.sender,
166169
&res.ctx.Lock,
@@ -173,6 +176,7 @@ func newEnvironment(t *testing.T, f upgradetest.Fork) *environment { //nolint:un
173176

174177
res.Builder = New(
175178
res.mempool,
179+
nil,
176180
&res.backend,
177181
res.blkManager,
178182
)

vms/platformvm/block/executor/backend.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ import (
1212
"github.com/ava-labs/avalanchego/utils/set"
1313
"github.com/ava-labs/avalanchego/vms/platformvm/block"
1414
"github.com/ava-labs/avalanchego/vms/platformvm/state"
15-
"github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool"
15+
"github.com/ava-labs/avalanchego/vms/platformvm/txs"
16+
"github.com/ava-labs/avalanchego/vms/txs/mempool"
1617
)
1718

1819
var errConflictingParentTxs = errors.New("block contains a transaction that conflicts with a transaction in a parent block")
1920

2021
// Shared fields used by visitors.
2122
type backend struct {
22-
mempool.Mempool
23+
mempool.Mempool[*txs.Tx]
2324
// lastAccepted is the ID of the last block that had Accept() called on it.
2425
lastAccepted ids.ID
2526

vms/platformvm/block/executor/helpers_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ import (
4848
"github.com/ava-labs/avalanchego/vms/platformvm/validators/validatorstest"
4949
"github.com/ava-labs/avalanchego/vms/secp256k1fx"
5050
"github.com/ava-labs/avalanchego/wallet/chain/p/wallet"
51+
52+
txmempool "github.com/ava-labs/avalanchego/vms/txs/mempool"
5153
)
5254

5355
const (
@@ -81,7 +83,7 @@ type test struct {
8183

8284
type environment struct {
8385
blkManager Manager
84-
mempool mempool.Mempool
86+
mempool txmempool.Mempool[*txs.Tx]
8587
sender *enginetest.Sender
8688

8789
isBootstrapped *utils.Atomic[bool]
@@ -153,14 +155,15 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller, f upgradetest.Fork) *
153155
metrics := metrics.Noop
154156

155157
var err error
156-
res.mempool, err = mempool.New("mempool", registerer, nil)
158+
res.mempool, err = mempool.New("mempool", registerer)
157159
if err != nil {
158160
panic(fmt.Errorf("failed to create mempool: %w", err))
159161
}
160162

161163
if ctrl == nil {
162164
res.blkManager = NewManager(
163165
res.mempool,
166+
nil,
164167
metrics,
165168
res.state,
166169
res.backend,
@@ -170,6 +173,7 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller, f upgradetest.Fork) *
170173
} else {
171174
res.blkManager = NewManager(
172175
res.mempool,
176+
nil,
173177
metrics,
174178
res.mockedState,
175179
res.backend,

vms/platformvm/block/executor/manager.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,16 @@ import (
1010

1111
"github.com/ava-labs/avalanchego/ids"
1212
"github.com/ava-labs/avalanchego/snow/consensus/snowman"
13+
"github.com/ava-labs/avalanchego/snow/engine/common"
1314
"github.com/ava-labs/avalanchego/utils/set"
1415
"github.com/ava-labs/avalanchego/vms/platformvm/block"
1516
"github.com/ava-labs/avalanchego/vms/platformvm/metrics"
1617
"github.com/ava-labs/avalanchego/vms/platformvm/state"
1718
"github.com/ava-labs/avalanchego/vms/platformvm/txs"
1819
"github.com/ava-labs/avalanchego/vms/platformvm/txs/executor"
1920
"github.com/ava-labs/avalanchego/vms/platformvm/txs/fee"
20-
"github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool"
2121
"github.com/ava-labs/avalanchego/vms/platformvm/validators"
22+
"github.com/ava-labs/avalanchego/vms/txs/mempool"
2223
)
2324

2425
var (
@@ -51,7 +52,8 @@ type Manager interface {
5152
}
5253

5354
func NewManager(
54-
mempool mempool.Mempool,
55+
mempool mempool.Mempool[*txs.Tx],
56+
toEngine chan<- common.Message,
5557
metrics metrics.Metrics,
5658
s state.State,
5759
txExecutorBackend *executor.Backend,
@@ -76,6 +78,7 @@ func NewManager(
7678
},
7779
rejector: &rejector{
7880
backend: backend,
81+
toEngine: toEngine,
7982
addTxsToMempool: !txExecutorBackend.Config.PartialSyncPrimaryNetwork,
8083
},
8184
preferred: lastAccepted,

vms/platformvm/block/executor/rejector.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package executor
66
import (
77
"go.uber.org/zap"
88

9+
"github.com/ava-labs/avalanchego/snow/engine/common"
910
"github.com/ava-labs/avalanchego/vms/platformvm/block"
1011
)
1112

@@ -16,6 +17,7 @@ var _ block.Visitor = (*rejector)(nil)
1617
// being shutdown.
1718
type rejector struct {
1819
*backend
20+
toEngine chan<- common.Message
1921
addTxsToMempool bool
2022
}
2123

@@ -82,7 +84,14 @@ func (r *rejector) rejectBlock(b block.Block, blockType string) error {
8284
}
8385
}
8486

85-
r.Mempool.RequestBuildBlock(false /*=emptyBlockPermitted*/)
87+
if r.Mempool.Len() == 0 {
88+
return nil
89+
}
90+
91+
select {
92+
case r.toEngine <- common.PendingTxs:
93+
default:
94+
}
8695

8796
return nil
8897
}

vms/platformvm/block/executor/rejector_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func TestRejectBlock(t *testing.T) {
120120
blk, err := tt.newBlockFunc()
121121
require.NoError(err)
122122

123-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
123+
mempool, err := mempool.New("", prometheus.NewRegistry())
124124
require.NoError(err)
125125
state := state.NewMockState(ctrl)
126126
blkIDToState := map[ids.ID]*blockState{

vms/platformvm/block/executor/verifier_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func newTestVerifier(t testing.TB, c testVerifierConfig) *verifier {
7575
c.ValidatorFeeConfig = genesis.LocalParams.ValidatorFeeConfig
7676
}
7777

78-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
78+
mempool, err := mempool.New("", prometheus.NewRegistry())
7979
require.NoError(err)
8080

8181
var (
@@ -454,7 +454,7 @@ func TestVerifierVisitCommitBlock(t *testing.T) {
454454

455455
// Create mocked dependencies.
456456
s := state.NewMockState(ctrl)
457-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
457+
mempool, err := mempool.New("", prometheus.NewRegistry())
458458
require.NoError(err)
459459
parentID := ids.GenerateTestID()
460460
parentStatelessBlk := block.NewMockBlock(ctrl)
@@ -528,7 +528,7 @@ func TestVerifierVisitAbortBlock(t *testing.T) {
528528

529529
// Create mocked dependencies.
530530
s := state.NewMockState(ctrl)
531-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
531+
mempool, err := mempool.New("", prometheus.NewRegistry())
532532
require.NoError(err)
533533
parentID := ids.GenerateTestID()
534534
parentStatelessBlk := block.NewMockBlock(ctrl)
@@ -603,7 +603,7 @@ func TestVerifyUnverifiedParent(t *testing.T) {
603603

604604
// Create mocked dependencies.
605605
s := state.NewMockState(ctrl)
606-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
606+
mempool, err := mempool.New("", prometheus.NewRegistry())
607607
require.NoError(err)
608608
parentID := ids.GenerateTestID()
609609

@@ -674,7 +674,7 @@ func TestBanffAbortBlockTimestampChecks(t *testing.T) {
674674

675675
// Create mocked dependencies.
676676
s := state.NewMockState(ctrl)
677-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
677+
mempool, err := mempool.New("", prometheus.NewRegistry())
678678
require.NoError(err)
679679
parentID := ids.GenerateTestID()
680680
parentStatelessBlk := block.NewMockBlock(ctrl)
@@ -775,7 +775,7 @@ func TestBanffCommitBlockTimestampChecks(t *testing.T) {
775775

776776
// Create mocked dependencies.
777777
s := state.NewMockState(ctrl)
778-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
778+
mempool, err := mempool.New("", prometheus.NewRegistry())
779779
require.NoError(err)
780780
parentID := ids.GenerateTestID()
781781
parentStatelessBlk := block.NewMockBlock(ctrl)
@@ -844,7 +844,7 @@ func TestVerifierVisitApricotStandardBlockWithProposalBlockParent(t *testing.T)
844844

845845
// Create mocked dependencies.
846846
s := state.NewMockState(ctrl)
847-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
847+
mempool, err := mempool.New("", prometheus.NewRegistry())
848848
require.NoError(err)
849849
parentID := ids.GenerateTestID()
850850
parentStatelessBlk := block.NewMockBlock(ctrl)
@@ -901,7 +901,7 @@ func TestVerifierVisitBanffStandardBlockWithProposalBlockParent(t *testing.T) {
901901

902902
// Create mocked dependencies.
903903
s := state.NewMockState(ctrl)
904-
mempool, err := mempool.New("", prometheus.NewRegistry(), nil)
904+
mempool, err := mempool.New("", prometheus.NewRegistry())
905905
require.NoError(err)
906906
parentID := ids.GenerateTestID()
907907
parentStatelessBlk := block.NewMockBlock(ctrl)

0 commit comments

Comments
 (0)