Skip to content

Commit d1f4125

Browse files
authored
perf: cleanup marshaling in batch (#2794)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. Ex: Closes #<issue number> --> This is a small change that avoids calling Hash() as we marshal in the function and then marshal a few lines below. I was fixing another issue and saw this. Its a minor optimisation that reduces allocations
1 parent b89d101 commit d1f4125

6 files changed

Lines changed: 37 additions & 8 deletions

File tree

block/internal/submitting/submitter.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,12 +391,13 @@ func (s *Submitter) IsHeightDAIncluded(height uint64, header *types.SignedHeader
391391
}
392392

393393
headerHash := header.Hash().String()
394-
dataHash := data.DACommitment().String()
394+
dataCommitment := data.DACommitment()
395+
dataHash := dataCommitment.String()
395396

396397
_, headerIncluded := s.cache.GetHeaderDAIncluded(headerHash)
397398
_, dataIncluded := s.cache.GetDataDAIncluded(dataHash)
398399

399-
dataIncluded = bytes.Equal(data.DACommitment(), common.DataHashForEmptyTxs) || dataIncluded
400+
dataIncluded = bytes.Equal(dataCommitment, common.DataHashForEmptyTxs) || dataIncluded
400401

401402
return headerIncluded && dataIncluded, nil
402403
}

block/internal/syncing/syncer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error {
418418
data := event.Data
419419
nextHeight := event.Header.Height()
420420
currentState := s.GetLastState()
421+
headerHash := header.Hash().String()
421422

422423
s.logger.Info().Uint64("height", nextHeight).Msg("syncing block")
423424

@@ -426,7 +427,7 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error {
426427
// The header validation must be done before applying the block to avoid executing gibberish
427428
if err := s.validateBlock(header, data); err != nil {
428429
// remove header as da included (not per se needed, but keep cache clean)
429-
s.cache.RemoveHeaderDAIncluded(header.Hash().String())
430+
s.cache.RemoveHeaderDAIncluded(headerHash)
430431
return errors.Join(errInvalidBlock, fmt.Errorf("failed to validate block: %w", err))
431432
}
432433

@@ -467,7 +468,7 @@ func (s *Syncer) trySyncNextBlock(event *common.DAHeightEvent) error {
467468
s.metrics.Height.Set(float64(newState.LastBlockHeight))
468469

469470
// Mark as seen
470-
s.cache.SetHeaderSeen(header.Hash().String(), header.Height())
471+
s.cache.SetHeaderSeen(headerHash, header.Height())
471472
if !bytes.Equal(header.DataHash, common.DataHashForEmptyTxs) {
472473
s.cache.SetDataSeen(data.DACommitment().String(), newState.LastBlockHeight)
473474
}

pkg/store/batch.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ func (b *DefaultBatch) SetHeight(height uint64) error {
4747

4848
// SaveBlockData saves block data to the batch
4949
func (b *DefaultBatch) SaveBlockData(header *types.SignedHeader, data *types.Data, signature *types.Signature) error {
50-
hash := header.Hash()
5150
height := header.Height()
5251
signatureHash := *signature
5352

@@ -70,8 +69,9 @@ func (b *DefaultBatch) SaveBlockData(header *types.SignedHeader, data *types.Dat
7069
return fmt.Errorf("failed to put signature blob in batch: %w", err)
7170
}
7271

72+
headerHash := types.HeaderHash(headerBlob)
7373
heightBytes := encodeHeight(height)
74-
if err := b.batch.Put(b.ctx, ds.NewKey(getIndexKey(hash)), heightBytes); err != nil {
74+
if err := b.batch.Put(b.ctx, ds.NewKey(getIndexKey(headerHash)), heightBytes); err != nil {
7575
return fmt.Errorf("failed to put index key in batch: %w", err)
7676
}
7777

pkg/store/store.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ func (s *DefaultStore) Rollback(ctx context.Context, height uint64, aggregator b
228228
}
229229

230230
for currentHeight > height {
231-
header, err := s.GetHeader(ctx, currentHeight)
231+
// Get header blob directly to reuse for hash calculation
232+
headerBlob, err := s.db.Get(ctx, ds.NewKey(getHeaderKey(currentHeight)))
232233
if err != nil {
233234
return fmt.Errorf("failed to get header at height %d: %w", currentHeight, err)
234235
}
@@ -245,7 +246,8 @@ func (s *DefaultStore) Rollback(ctx context.Context, height uint64, aggregator b
245246
return fmt.Errorf("failed to delete signature of block blob in batch: %w", err)
246247
}
247248

248-
hash := header.Hash()
249+
// Use HeaderHash to avoid re-marshaling the header
250+
hash := types.HeaderHash(headerBlob)
249251
if err := batch.Delete(ctx, ds.NewKey(getIndexKey(hash))); err != nil {
250252
return fmt.Errorf("failed to delete index key in batch: %w", err)
251253
}

types/hashing.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ func (h *Header) Hash() Hash {
1515
if err != nil {
1616
return nil
1717
}
18+
return HeaderHash(bytes)
19+
}
20+
21+
// HeaderHash returns the SHA256 hash of pre-marshaled header bytes.
22+
// Use this function when you already have marshaled header bytes to avoid
23+
// redundant marshaling operations. For convenience, use Header.Hash() instead.
24+
func HeaderHash(bytes []byte) Hash {
1825
hash := sha256.Sum256(bytes)
1926
return hash[:]
2027
}

types/hashing_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,21 @@ func TestLeafHashOpt(t *testing.T) {
9696
assert.Equal(t, expectedHash2, hash2)
9797
assert.NotEqual(t, hash1, hash2)
9898
}
99+
100+
// TestHeaderHashWithBytes tests the HeaderHash function directly
101+
func TestHeaderHashWithBytes(t *testing.T) {
102+
header := &Header{
103+
BaseHeader: BaseHeader{Height: 1, Time: 1234567890},
104+
DataHash: []byte("datahash"),
105+
}
106+
107+
// Hash using the method
108+
hash1 := header.Hash()
109+
110+
// Hash using the function directly
111+
headerBytes, err := header.MarshalBinary()
112+
require.NoError(t, err)
113+
hash2 := HeaderHash(headerBytes)
114+
115+
assert.Equal(t, hash1, hash2, "HeaderHash should produce same result as Header.Hash()")
116+
}

0 commit comments

Comments
 (0)