Skip to content

Commit fa8143f

Browse files
tac0turtleclaude[bot]tac0turtleMarko
authored
fix: eliminate gas price/multiplier duplication (#2539)
closes #2316 This PR eliminates the duplication of gas price and gas multiplier by using the DA interface as the single source of truth. ## Changes - Remove `gasPrice` and `gasMultiplier` fields from Manager struct - Update `submitToDA` function to call DA interface methods - Remove gas parameters from `NewManager` constructor - Update tests to mock DA interface methods - Add proper error handling with fallback defaults ## Benefits - Single source of truth for gas parameters - Enables dynamic gas pricing by DA implementations - Reduces configuration complexity - Maintains backward compatibility Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Marko <[email protected]> Co-authored-by: tac0turtle <[email protected]> Co-authored-by: Marko <[email protected]>
1 parent aa69f8a commit fa8143f

File tree

12 files changed

+115
-37
lines changed

12 files changed

+115
-37
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Ev-node is the basis of the Evolve Stack. For more in-depth information about Ev
1111
## Using Evolve
1212

1313
Evolve supports multiple sync modes:
14+
1415
- **Hybrid sync**: Sync from both DA layer and P2P network (default when peers are configured)
1516
- **DA-only sync**: Sync exclusively from DA layer by leaving P2P peers empty (see [Configuration Guide](docs/learn/config.md#da-only-sync-mode))
1617
- **P2P-priority sync**: Prioritize P2P with DA as fallback

block/CLAUDE.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ The block package is the core of ev-node's block management system. It handles b
99
## Core Components
1010

1111
### Manager (`manager.go`)
12+
1213
- **Purpose**: Central orchestrator for all block operations
1314
- **Key Responsibilities**:
1415
- Transaction aggregation into blocks
@@ -18,6 +19,7 @@ The block package is the core of ev-node's block management system. It handles b
1819
- P2P block/header gossiping
1920

2021
### Aggregation (`aggregation.go`, `lazy_aggregation_test.go`)
22+
2123
- **Purpose**: Collects transactions from mempool and creates blocks
2224
- **Modes**:
2325
- **Normal Mode**: Produces blocks at regular intervals (BlockTime)
@@ -28,6 +30,7 @@ The block package is the core of ev-node's block management system. It handles b
2830
- `normalAggregationLoop`: Regular block production
2931

3032
### Synchronization (`sync.go`, `sync_test.go`)
33+
3134
- **Purpose**: Keeps the node synchronized with the network
3235
- **Key Functions**:
3336
- `SyncLoop`: Main synchronization loop
@@ -36,6 +39,7 @@ The block package is the core of ev-node's block management system. It handles b
3639
- Handles header and data caching
3740

3841
### Data Availability (`da_includer.go`, `submitter.go`, `retriever.go`)
42+
3943
- **DA Includer**: Manages DA blob inclusion proofs and validation
4044
- **Submitter**: Handles block submission to the DA layer with retry logic
4145
- **Retriever**: Fetches blocks from the DA layer
@@ -45,6 +49,7 @@ The block package is the core of ev-node's block management system. It handles b
4549
- Batch submission optimization
4650

4751
### Storage (`store.go`, `store_test.go`)
52+
4853
- **Purpose**: Persistent storage for blocks and state
4954
- **Key Features**:
5055
- Block height tracking
@@ -53,6 +58,7 @@ The block package is the core of ev-node's block management system. It handles b
5358
- Migration support for namespace changes
5459

5560
### Pending Blocks (`pending_base.go`, `pending_headers.go`, `pending_data.go`)
61+
5662
- **Purpose**: Manages blocks awaiting DA inclusion or validation
5763
- **Components**:
5864
- **PendingBase**: Base structure for pending blocks
@@ -64,6 +70,7 @@ The block package is the core of ev-node's block management system. It handles b
6470
- Memory-efficient caching
6571

6672
### Metrics (`metrics.go`, `metrics_helpers.go`)
73+
6774
- **Purpose**: Performance monitoring and observability
6875
- **Key Metrics**:
6976
- Block production times
@@ -74,20 +81,23 @@ The block package is the core of ev-node's block management system. It handles b
7481
## Key Workflows
7582

7683
### Block Production Flow
84+
7785
1. Transactions collected from mempool
7886
2. Block created with proper header and data
7987
3. Block executed through executor
8088
4. Block submitted to DA layer
8189
5. Block gossiped to P2P network
8290

8391
### Synchronization Flow
92+
8493
1. Headers received from P2P network
8594
2. Headers validated and cached
8695
3. Block data retrieved from DA layer
8796
4. Blocks applied to state
8897
5. Sync progress updated
8998

9099
### DA Submission Flow
100+
91101
1. Block prepared for submission
92102
2. Blob created with block data
93103
3. Submission attempted with retries
@@ -97,47 +107,55 @@ The block package is the core of ev-node's block management system. It handles b
97107
## Configuration
98108

99109
### Time Parameters
110+
100111
- `BlockTime`: Target time between blocks (default: 1s)
101112
- `DABlockTime`: DA layer block time (default: 6s)
102113
- `LazyBlockTime`: Max time between blocks in lazy mode (default: 60s)
103114

104115
### Limits
116+
105117
- `maxSubmitAttempts`: Max DA submission retries (30)
106118
- `defaultMempoolTTL`: Blocks until tx dropped (25)
107119

108120
## Testing Strategy
109121

110122
### Unit Tests
123+
111124
- Test individual components in isolation
112125
- Mock external dependencies (DA, executor, sequencer)
113126
- Focus on edge cases and error conditions
114127

115128
### Integration Tests
129+
116130
- Test component interactions
117131
- Verify block flow from creation to storage
118132
- Test synchronization scenarios
119133

120134
### Performance Tests (`da_speed_test.go`)
135+
121136
- Measure DA submission performance
122137
- Test batch processing efficiency
123138
- Validate metrics accuracy
124139

125140
## Common Development Tasks
126141

127142
### Adding a New DA Feature
143+
128144
1. Update DA interfaces in `core/da`
129145
2. Modify `da_includer.go` for inclusion logic
130146
3. Update `submitter.go` for submission flow
131147
4. Add retrieval logic in `retriever.go`
132148
5. Update tests and metrics
133149

134150
### Modifying Block Production
151+
135152
1. Update aggregation logic in `aggregation.go`
136153
2. Adjust timing in Manager configuration
137154
3. Update metrics collection
138155
4. Test both normal and lazy modes
139156

140157
### Implementing New Sync Strategy
158+
141159
1. Modify `SyncLoop` in `sync.go`
142160
2. Update pending block handling
143161
3. Adjust cache strategies
@@ -182,4 +200,4 @@ The block package is the core of ev-node's block management system. It handles b
182200
- Log with structured fields
183201
- Return errors with context
184202
- Use metrics for observability
185-
- Test error conditions thoroughly
203+
- Test error conditions thoroughly

block/da_includer.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,12 @@ func (m *Manager) incrementDAIncludedHeight(ctx context.Context) error {
7474

7575
// Update sequencer metrics if the sequencer supports it
7676
if seq, ok := m.sequencer.(MetricsRecorder); ok {
77-
seq.RecordMetrics(m.gasPrice, 0, coreda.StatusSuccess, m.pendingHeaders.numPendingHeaders(), newHeight)
77+
gasPrice, err := m.da.GasPrice(ctx)
78+
if err != nil {
79+
m.logger.Warn().Err(err).Msg("failed to get gas price from DA layer, using default for metrics")
80+
gasPrice = 0.0 // Use default gas price for metrics
81+
}
82+
seq.RecordMetrics(gasPrice, 0, coreda.StatusSuccess, m.pendingHeaders.numPendingHeaders(), newHeight)
7883
}
7984

8085
return nil

block/da_includer_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,10 +421,14 @@ func TestIncrementDAIncludedHeight_WithMetricsRecorder(t *testing.T) {
421421
expectedDAIncludedHeight := startDAIncludedHeight + 1
422422
m.daIncludedHeight.Store(startDAIncludedHeight)
423423

424+
// Set up mock DA
425+
mockDA := mocks.NewMockDA(t)
426+
mockDA.On("GasPrice", mock.Anything).Return(1.5, nil).Once()
427+
m.da = mockDA
428+
424429
// Set up mock sequencer with metrics
425430
mockSequencer := new(MockSequencerWithMetrics)
426431
m.sequencer = mockSequencer
427-
m.gasPrice = 1.5 // Set a test gas price
428432

429433
// Mock the store calls needed for PendingHeaders initialization
430434
// First, clear the existing Height mock from newTestManager
@@ -462,6 +466,7 @@ func TestIncrementDAIncludedHeight_WithMetricsRecorder(t *testing.T) {
462466
store.AssertExpectations(t)
463467
exec.AssertExpectations(t)
464468
mockSequencer.AssertExpectations(t)
469+
mockDA.AssertExpectations(t)
465470
}
466471

467472
// Note: It is not practical to unit test a CompareAndSwap failure for incrementDAIncludedHeight

block/manager.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,6 @@ type Manager struct {
144144
// in the DA
145145
daIncludedHeight atomic.Uint64
146146
da coreda.DA
147-
gasPrice float64
148-
gasMultiplier float64
149147

150148
sequencer coresequencer.Sequencer
151149
lastBatchData [][]byte
@@ -303,8 +301,6 @@ func NewManager(
303301
headerBroadcaster broadcaster[*types.SignedHeader],
304302
dataBroadcaster broadcaster[*types.Data],
305303
seqMetrics *Metrics,
306-
gasPrice float64,
307-
gasMultiplier float64,
308304
managerOpts ManagerOptions,
309305
) (*Manager, error) {
310306
s, err := getInitialState(ctx, genesis, signer, store, exec, logger, managerOpts)
@@ -395,8 +391,6 @@ func NewManager(
395391
sequencer: sequencer,
396392
exec: exec,
397393
da: da,
398-
gasPrice: gasPrice,
399-
gasMultiplier: gasMultiplier,
400394
txNotifyCh: make(chan struct{}, 1), // Non-blocking channel
401395
signaturePayloadProvider: managerOpts.SignaturePayloadProvider,
402396
validatorHasherProvider: managerOpts.ValidatorHasherProvider,

block/manager_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ func getManager(t *testing.T, da da.DA, gasPrice float64, gasMultiplier float64)
4747
headerCache: cache.NewCache[types.SignedHeader](),
4848
dataCache: cache.NewCache[types.Data](),
4949
logger: logger,
50-
gasPrice: gasPrice,
51-
gasMultiplier: gasMultiplier,
5250
lastStateMtx: &sync.RWMutex{},
5351
metrics: NopMetrics(),
5452
store: mockStore,
@@ -212,6 +210,7 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) {
212210
ctx := context.Background()
213211

214212
mockDA := mocks.NewMockDA(t)
213+
mockDA.On("GasPrice", mock.Anything).Return(1.0, nil).Maybe()
215214
m, _ := getManager(t, mockDA, -1, -1)
216215

217216
header1, data1 := types.GetRandomBlock(uint64(1), 5, chainID)
@@ -250,6 +249,7 @@ func Test_submitBlocksToDA_BlockMarshalErrorCase2(t *testing.T) {
250249
ctx := context.Background()
251250

252251
mockDA := mocks.NewMockDA(t)
252+
mockDA.On("GasPrice", mock.Anything).Return(1.0, nil).Maybe()
253253
m, _ := getManager(t, mockDA, -1, -1)
254254

255255
header1, data1 := types.GetRandomBlock(uint64(1), 5, chainID)

block/publish_block_p2p_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,6 @@ func setupBlockManager(t *testing.T, ctx context.Context, workDir string, mainKV
215215
nil,
216216
nil,
217217
NopMetrics(),
218-
1.,
219-
1.,
220218
DefaultManagerOptions(),
221219
)
222220
require.NoError(t, err)

block/submitter.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,23 @@ import (
1111
)
1212

1313
const (
14-
submissionTimeout = 60 * time.Second
15-
noGasPrice = -1
16-
initialBackoff = 100 * time.Millisecond
14+
submissionTimeout = 60 * time.Second
15+
noGasPrice = -1
16+
initialBackoff = 100 * time.Millisecond
17+
defaultGasPrice = 0.0
18+
defaultGasMultiplier = 1.0
1719
)
1820

21+
// getGasMultiplier fetches the gas multiplier from DA layer with fallback to default value
22+
func (m *Manager) getGasMultiplier(ctx context.Context) float64 {
23+
gasMultiplier, err := m.da.GasMultiplier(ctx)
24+
if err != nil {
25+
m.logger.Warn().Err(err).Msg("failed to get gas multiplier from DA layer, using default")
26+
return defaultGasMultiplier
27+
}
28+
return gasMultiplier
29+
}
30+
1931
// retryStrategy manages retry logic with backoff and gas price adjustments for DA submissions
2032
type retryStrategy struct {
2133
attempt int
@@ -221,7 +233,13 @@ func submitToDA[T any](
221233
return err
222234
}
223235

224-
retryStrategy := newRetryStrategy(m.gasPrice, m.config.DA.BlockTime.Duration, m.config.DA.MaxSubmitAttempts)
236+
gasPrice, err := m.da.GasPrice(ctx)
237+
if err != nil {
238+
m.logger.Warn().Err(err).Msg("failed to get gas price from DA layer, using default")
239+
gasPrice = defaultGasPrice
240+
}
241+
242+
retryStrategy := newRetryStrategy(gasPrice, m.config.DA.BlockTime.Duration, m.config.DA.MaxSubmitAttempts)
225243
remaining := items
226244
numSubmitted := 0
227245

@@ -293,10 +311,10 @@ func handleSubmissionResult[T any](
293311
) submissionOutcome[T] {
294312
switch res.Code {
295313
case coreda.StatusSuccess:
296-
return handleSuccessfulSubmission(m, remaining, marshaled, &res, postSubmit, retryStrategy, itemType)
314+
return handleSuccessfulSubmission(ctx, m, remaining, marshaled, &res, postSubmit, retryStrategy, itemType)
297315

298316
case coreda.StatusNotIncludedInBlock, coreda.StatusAlreadyInMempool:
299-
return handleMempoolFailure(m, &res, retryStrategy, retryStrategy.attempt, remaining, marshaled)
317+
return handleMempoolFailure(ctx, m, &res, retryStrategy, retryStrategy.attempt, remaining, marshaled)
300318

301319
case coreda.StatusContextCanceled:
302320
m.logger.Info().Int("attempt", retryStrategy.attempt).Msg("DA layer submission canceled due to context cancellation")
@@ -315,6 +333,7 @@ func handleSubmissionResult[T any](
315333
}
316334

317335
func handleSuccessfulSubmission[T any](
336+
ctx context.Context,
318337
m *Manager,
319338
remaining []T,
320339
marshaled [][]byte,
@@ -335,7 +354,10 @@ func handleSuccessfulSubmission[T any](
335354
notSubmittedMarshaled := marshaled[res.SubmittedCount:]
336355

337356
postSubmit(submitted, res, retryStrategy.gasPrice)
338-
retryStrategy.ResetOnSuccess(m.gasMultiplier)
357+
358+
gasMultiplier := m.getGasMultiplier(ctx)
359+
360+
retryStrategy.ResetOnSuccess(gasMultiplier)
339361

340362
m.logger.Debug().Dur("backoff", retryStrategy.backoff).Float64("gasPrice", retryStrategy.gasPrice).Msg("resetting DA layer submission options")
341363

@@ -349,6 +371,7 @@ func handleSuccessfulSubmission[T any](
349371
}
350372

351373
func handleMempoolFailure[T any](
374+
ctx context.Context,
352375
m *Manager,
353376
res *coreda.ResultSubmit,
354377
retryStrategy *retryStrategy,
@@ -359,8 +382,9 @@ func handleMempoolFailure[T any](
359382
m.logger.Error().Str("error", res.Message).Int("attempt", attempt).Msg("DA layer submission failed")
360383

361384
m.recordDAMetrics("submission", DAModeFail)
362-
retryStrategy.BackoffOnMempool(int(m.config.DA.MempoolTTL), m.config.DA.BlockTime.Duration, m.gasMultiplier)
363385

386+
gasMultiplier := m.getGasMultiplier(ctx)
387+
retryStrategy.BackoffOnMempool(int(m.config.DA.MempoolTTL), m.config.DA.BlockTime.Duration, gasMultiplier)
364388
m.logger.Info().Dur("backoff", retryStrategy.backoff).Float64("gasPrice", retryStrategy.gasPrice).Msg("retrying DA layer submission with")
365389

366390
return submissionOutcome[T]{
@@ -402,7 +426,8 @@ func handleTooBigError[T any](
402426
if totalSubmitted > 0 {
403427
newRemaining := remaining[totalSubmitted:]
404428
newMarshaled := marshaled[totalSubmitted:]
405-
retryStrategy.ResetOnSuccess(m.gasMultiplier)
429+
gasMultiplier := m.getGasMultiplier(ctx)
430+
retryStrategy.ResetOnSuccess(gasMultiplier)
406431

407432
return submissionOutcome[T]{
408433
RemainingItems: newRemaining,

0 commit comments

Comments
 (0)