tbc: add lazy block reader for zero-copy per-tx access#1051
tbc: add lazy block reader for zero-copy per-tx access#1051marcopeereboom wants to merge 1 commit into
Conversation
github-actions
Bot
added
area: tbc
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
BlockByHash calls btcutil.NewBlockFromBytes on every read, eagerly deserializing the entire block into heap objects. Callers needing one tx pay for all txs. CPU profiles show 50% GC pressure from this. Add lazyBlock type wrapping raw []byte from the block cache with lazy per-tx access — no deserialization until a specific tx is requested. Single-pass boundary scan finds tx offsets without parsing. Per-tx txid computation handles both witness and non-witness serialization. Per-tx output value extraction reads only the outputs section. Add BlockRawByHash to the DB interface — the cache-check + LevelDB read path from BlockByHash without the NewBlockFromBytes call. Existing BlockByHash callers are unchanged — this is an opt-in parallel path for callers that need lightweight access. 100% test coverage on lazyblock.go. Every method cross-checked against btcutil.NewBlockFromBytes output as oracle. Test blocks include: genesis, segwit single/multi input, mixed segwit/non-segwit, 50-tx blocks, empty witness items, large inscription-style witness, FullBlock byte-identical round-trip, and exhaustive error path coverage for all truncation boundaries.
marcopeereboom
force-pushed
the
marco/lazy-block
branch
from
9f1a394 to
99d768f
Compare
github-actions
Bot
added
area: docs
| // get from cache | ||
| var ( | ||
| eb []byte | ||
| err error | ||
| ) | ||
| if l.cfg.blockCacheSize > 0 { | ||
| eb, _ = l.blockCache.Get(hash) | ||
| } | ||
|
|
||
| // get from db | ||
| if eb == nil { | ||
| bDB := l.rawPool[level.BlocksDB] | ||
| eb, err = bDB.Get(hash[:]) | ||
| if err != nil { | ||
| if errors.Is(err, leveldb.ErrNotFound) { | ||
| return nil, database.BlockNotFoundError{Hash: hash} | ||
| } | ||
| return nil, fmt.Errorf("block raw get: %w", err) | ||
| } | ||
| if l.cfg.blockCacheSize > 0 { | ||
| l.blockCache.Put(hash, eb) | ||
| } | ||
| } | ||
|
|
||
| return eb, nil |
There was a problem hiding this comment.
nit: I think this could be made more tidy by making the first cache check a quick-return instead of always falling through to a nil check. e.g.
| // get from cache | |
| var ( | |
| eb []byte | |
| err error | |
| ) | |
| if l.cfg.blockCacheSize > 0 { | |
| eb, _ = l.blockCache.Get(hash) | |
| } | |
| // get from db | |
| if eb == nil { | |
| bDB := l.rawPool[level.BlocksDB] | |
| eb, err = bDB.Get(hash[:]) | |
| if err != nil { | |
| if errors.Is(err, leveldb.ErrNotFound) { | |
| return nil, database.BlockNotFoundError{Hash: hash} | |
| } | |
| return nil, fmt.Errorf("block raw get: %w", err) | |
| } | |
| if l.cfg.blockCacheSize > 0 { | |
| l.blockCache.Put(hash, eb) | |
| } | |
| } | |
| return eb, nil | |
| // get from cache | |
| if l.cfg.blockCacheSize > 0 { | |
| if eb, _ := l.blockCache.Get(hash); eb != nil { | |
| return eb, nil | |
| } | |
| } | |
| // get from db | |
| bDB := l.rawPool[level.BlocksDB] | |
| eb, err := bDB.Get(hash[:]) | |
| if err != nil { | |
| if errors.Is(err, leveldb.ErrNotFound) { | |
| return nil, database.BlockNotFoundError{Hash: hash} | |
| } | |
| return nil, fmt.Errorf("block raw get: %w", err) | |
| } | |
| if l.cfg.blockCacheSize > 0 { | |
| l.blockCache.Put(hash, eb) | |
| } | |
| return eb, nil |
| // must be a complete serialized Bitcoin block (header + transactions). | ||
| // No data is parsed until a method is called. | ||
| func newLazyBlock(raw []byte) *lazyBlock { | ||
| return &lazyBlock{raw: raw} |
There was a problem hiding this comment.
Consider validating the length of raw once here, to prevent a lazyBlock from being created wrapping obviously incorrect raw bytes. This also removes the need to check the length in receivers.
Related: https://github.com/hemilabs/heminetwork/pull/1051/changes#r3323667033
| return nil, nil, fmt.Errorf("scanTxBoundaries: tx %d input %d: script len: %w", t, i, err) | ||
| } | ||
| offset += n | ||
| if offset+int(scriptLen) > len(raw) { |
There was a problem hiding this comment.
It is unsafe to cast scriptLen to an int, as readVarInt explicitly parses untrusted bytes as a uint64 and returns it as such.
With a valid block, it would not be possible to trigger a panic - but it is still possible on invalid or corrupted raw inputs. I think it would be better to handle the value correctly than make assumptions.
Same on lines 231, 261, and 264.
| if offset+int(scriptLen) > len(raw) { | |
| if scriptLen > uint64(len(raw) - offset) { |
Related: https://github.com/hemilabs/heminetwork/pull/1051/changes#r3323667033
| locs := make([]wire.TxLoc, 0, txCount) | ||
| witness := make([]bool, 0, txCount) |
There was a problem hiding this comment.
While not possible with valid Bitcoin block, txCount is a uint64 parsed from untrusted bytes. This could potentially allow unbounded allocation of these slices, up-to triggering a panic.
Consider validating txCount before allocating.
Related: https://github.com/hemilabs/heminetwork/pull/1051/changes#r3323667033
| } | ||
| offset += n | ||
|
|
||
| values := make([]uint64, outputCount) |
There was a problem hiding this comment.
While not exploitable with a valid Bitcoin block, outputCount is a uint64 parsed from untrusted bytes. This could potentially allow unbounded allocation of these slices, up-to triggering a panic.
Consider validating outputCount before allocating.
Related: https://github.com/hemilabs/heminetwork/pull/1051/changes#r3323667033
| offsets, witness, err := scanTxBoundaries(lb.raw) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| lb.txOffsets = offsets | ||
| lb.txWitness = witness | ||
| return nil |
There was a problem hiding this comment.
| offsets, witness, err := scanTxBoundaries(lb.raw) | |
| if err != nil { | |
| return err | |
| } | |
| lb.txOffsets = offsets | |
| lb.txWitness = witness | |
| return nil | |
| var err error | |
| lb.txOffsets, lb.txWitness, err = scanTxBoundaries(lb.raw) | |
| return err |
|
|
||
| // newLazyBlock wraps raw block bytes for lazy access. The raw slice | ||
| // must be a complete serialized Bitcoin block (header + transactions). | ||
| // No data is parsed until a method is called. |
There was a problem hiding this comment.
Consider documenting expectations for raw input here.
The correctness of lazyBlock is entirely dependant on raw being guaranteed as immutable and kept alive for the full lifetime of every lazyBlock. If the underlying raw byte slice is ever reused or recycled, the entire lazyBlock would be silently corrupted and later cause errors, panics or incorrect data to be returned.
Additionally, if raw is a sub-slice of a larger byte slice, holding it pins the entire parent's backing array, not just the block's own len(raw) bytes. The raw slice would have to be copied into a new slice to prevent this.
This also assumes raw is always a valid Bitcoin block, which is not guaranteed. If created with incorrect or corrupted data, receivers below can error, panic or return invalid data. - If all instances of a lazyBlock being created are guaranteed to only input valid data, this is probably okay; if not, the scanning needs hardening.
| // No data is parsed until a method is called. | |
| // No data is parsed until a method is called. | |
| // The caller must guarantee that raw is not mutated, reused, or recycled | |
| // for the lifetime of the returned lazyBlock. raw bytes are referenced, | |
| // not copied. raw must be a complete, valid serialized Bitcoin block. |
2 participants
Summary
Add a
lazyBlocktype that wraps raw block bytes from the block cache with lazy per-tx access. Nothing is parsed until explicitly requested — individual tx access parses only that tx from its raw byte range.Problem
BlockByHashcallsbtcutil.NewBlockFromByteson every read, which eagerly deserializes the entire block into heap objects (MsgBlock, everyMsgTx, everyTxIn/TxOut, witness slices). For a 3 MB block with 5000 txs, callers that need one tx pay for all 5000. CPU profile shows 50%+ GC pressure from discarded deserialization objects.Solution
BlockRawByHashadded to DB interface — returns raw[]bytefrom block cache/LevelDB without parsinglazyBlocktype inservice/tbc/lazyblock.go:Hash()— SHA256 of first 80 bytes, computed on demandTxCount()/TxHash(i)— scan tx boundaries from raw bytes, compute txid via SHA256 (handles segwit witness flag correctly)FindTx(txid)— iterateTxHash(i)until matchTxOutputValues(i)— parse only one tx's output values from raw bytesFullBlock()— fallback tobtcutil.NewBlockFromBytesNo btcutil fork. External to btcutil entirely. Existing
BlockByHashcallers unchanged.Testing
Files changed
database/tbcd/database.go— addBlockRawByHashto interfacedatabase/tbcd/level/level.go— implementBlockRawByHashservice/tbc/lazyblock.go— new fileservice/tbc/lazyblock_test.go— new fileservice/tbc/cpfp_test.go— stub updated