Skip to content

Suggest different design for ADR 21 #2242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tac0turtle merged 11 commits into from
May 2, 2025
Merged

Suggest different design for ADR 21 #2242

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b584139
add a proposal for lazy
May 2, 2025
6497742
linting
May 2, 2025
ad16e64
Suggest different design
Manav-Aggarwal May 2, 2025
0fa4828
fix how available is spelled
Manav-Aggarwal May 2, 2025
dcb2d04
Remove checkForTransactions
Manav-Aggarwal May 2, 2025
58ad3d1
Remove producing block on txNotifyCh
Manav-Aggarwal May 2, 2025
e1ed8cc
add timer resets
Manav-Aggarwal May 2, 2025
d3b93aa
use produce_block
Manav-Aggarwal May 2, 2025
eae0650
fix tests
May 2, 2025
0b351c4
amend adr
May 2, 2025
8f1ffae
Merge branch 'marko/adr-21-impl' into manav/suggest-diff-design-adr-21
tac0turtle May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
linting
  • Loading branch information
tac0turtle
tac0turtle committed May 2, 2025
commit 64977427f6cd9e76c517d2fe7b617df1885b9810
9 changes: 6 additions & 3 deletions block/aggregation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ func (m *Manager) lazyAggregationLoop(ctx context.Context, blockTimer *time.Time

case <-blockTimer.C:
m.logger.Debug("Block timer triggered block production")
// m.produceBlock(ctx, "block_timer", lazyTimer, blockTimer)
// Block production is intentionally disabled for the blockTimer case in lazy mode.
// This is because lazy mode prioritizes transaction-driven or periodic triggers
// (e.g., lazyTimer) over time-based triggers to optimize resource usage.

case <-m.txNotifyCh:
// Only proceed if we're not being throttled
Expand Down Expand Up @@ -126,7 +128,8 @@ func (m *Manager) normalAggregationLoop(ctx context.Context, blockTimer *time.Ti
blockTimer.Reset(getRemainingSleep(start, m.config.Node.BlockTime.Duration))

case <-m.txNotifyCh:
// clear notification channel
// Transaction notifications are intentionally ignored in normal mode
// to avoid triggering block production outside the scheduled intervals.
}
}
}
Expand Down Expand Up @@ -171,7 +174,7 @@ func (m *Manager) checkForTransactions(ctx context.Context) (bool, error) {
}

// If we got a batch with transactions, return true
if batchData != nil && batchData.Batch != nil && len(batchData.Batch.Transactions) > 0 {
if batchData != nil && batchData.Batch != nil && len(batchData.Transactions) > 0 {
return true, nil
}

Expand Down
10 changes: 10 additions & 0 deletions block/lazy_aggregation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ type mockPublishBlock struct {
delay time.Duration // Optional delay to simulate processing time
}

// reset clears the calls channel in mockPublishBlock.
func (m *mockPublishBlock) reset() {
m.mu.Lock()
defer m.mu.Unlock()
// Clear the channel
for len(m.calls) > 0 {
<-m.calls
}
}

func (m *mockPublishBlock) publish(ctx context.Context) error {
m.mu.Lock()
err := m.err
Expand Down
3 changes: 0 additions & 3 deletions specs/lazy-adr/adr-007-header-commit-to-shares.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,10 @@ Discarded

### Positive


### Negative


### Neutral


## References

1. <https://github.com/rollkit/rollkit/issues/133>
9 changes: 5 additions & 4 deletions specs/lazy-adr/adr-021-lazy-aggregation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ Leverage the existing empty batch mechanism and `dataHashForEmptyTxs` to maintai
1. **Modified Batch Retrieval**:

```go

func (m *Manager) retrieveBatch(ctx context.Context) (*BatchData, error) {
res, err := m.sequencer.GetBatch(ctx)
if err != nil {
return nil, err
}
res, err := m.sequencer.GetBatch(ctx)
if err != nil {
return nil, err
}

if res != nil && res.Batch != nil {
// Even if there are no transactions, return the batch with timestamp
Expand Down