feat: add TimeNano field for nanosecond block timestamps - #2253
Conversation
…stamps Add nanosecond-precision block timestamp capture via new TimeNano field in BlockExtraData. This enables tracking precise block propagation timing beyond the second-precision header.Time field. Changes: - Add TimeNano field to BlockExtraData (RLP optional) - Add GetTimeNano() helper on Header - Add Placeholder hardfork to gate TimeNano validation - Validate TimeNano presence in verifyHeader for Placeholder+ blocks - Restructure Prepare to calculate timestamps before BlockExtraData encoding The Placeholder hardfork name is temporary and will be renamed.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (85.71%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2253 +/- ##
========================================
Coverage 55.23% 55.23%
========================================
Files 912 912
Lines 165870 165883 +13
========================================
+ Hits 91616 91631 +15
+ Misses 68803 68790 -13
- Partials 5451 5462 +11
... and 27 files with indirect coverage changes
🚀 New features to boost your workflow:
|
|
|
This should be reviewed alongside: #2277 |
|
codegenie review |
🧞 Codegenie ReviewReviewed all 15 hunks (11 deep, 2 normal, 2 light); no hunks skipped or failed. Three verified findings, all tied to the new The main one is a producer/consumer mismatch in The other two are lower severity: the new Not covered by verified findings, worth confirming during merge: whether a test asserts CoverageReviewed 15/15 hunks.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧞 Codegenie Review
Reviewed all 15 hunks (11 deep, 2 normal, 2 light); no hunks skipped or failed. Three verified findings, all tied to the new TimeNano extra-data field.
The main one is a producer/consumer mismatch in core/types/block.go: BlockExtraData gained a fifth RLP-optional field, but the internal mirror struct blockExtraDataRawTxDeps used by the fast header-decode path still declares four, so pre-Austin headers carrying TimeNano fail to decode and GetValidatorBytes/GetBaseFeeParams return nil. This only fires on configs where Hampi is active at heights where Austin is not, so the author should either add the field or confirm the fork-ordering invariant.
The other two are lower severity: the new verifyHeader Hampi check decodes header.Extra a second time per header (and on the pre-Austin branch expands TxDependency), and the added tests do not exercise sub-second TimeNano precision through Prepare.
Not covered by verified findings, worth confirming during merge: whether a test asserts errMissingTimeNano for a post-Hampi header lacking TimeNano, and whether hampiChainConfig adds Hampi-specific ChainConfig gating or relies solely on BorConfig.HampiBlock.
Reviewed 15/15 hunks.
Coverage levels: deep 11, normal 2, light 2, skip 0.
— codegenie v0.5.6 (a662388fde) · View Workflow Job
| ValidatorBytes: validatorBytes, | ||
| GasTarget: gasTarget, | ||
| BaseFeeChangeDenominator: baseFeeChangeDenom, | ||
| TimeNano: timeNano, |
There was a problem hiding this comment.
BlockExtraData gained a fifth RLP field (TimeNano), but the internal mirror struct blockExtraDataRawTxDeps — which the fast header-decode path uses for the pre-Austin branch — still declares only four fields. Its own comment asserts wire-identity with BlockExtraData, and that invariant no longer holds.
Impact: On any chain config where Hampi is active at a height where Austin is not, Prepare emits a pre-Austin BlockExtraData RLP list with 5 elements. Header.decodeExtraFieldsFast takes the non-Austin branch, RLP decoding fails with rlp: input list has too many elements, and the error is only logged at Debug level before returning ok=false. GetValidatorBytes and GetBaseFeeParams then return nil, so header verification cannot obtain the validator set or base-fee params for blocks the node itself produced.
Encoder now emits the field on the pre-Austin shape:
return rlp.EncodeToBytes(&BlockExtraData{
ValidatorBytes: validatorBytes,
GasTarget: gasTarget,
BaseFeeChangeDenominator: baseFeeChangeDenom,
TimeNano: timeNano, // new 5th element in the pre-Austin shape
})The mirror struct in core/types/block.go (lines 164-170) was not updated:
// blockExtraDataRawTxDeps mirrors BlockExtraData but keeps TxDependency as an
// rlp.RawValue ... The wire format is identical to BlockExtraData.
type blockExtraDataRawTxDeps struct {
ValidatorBytes []byte
TxDependency rlp.RawValue
GasTarget *uint64 `rlp:"optional"`
BaseFeeChangeDenominator *uint64 `rlp:"optional"`
}And the decode path swallows the failure (core/types/block.go, ~line 557):
var blockExtraData blockExtraDataRawTxDeps
if err := rlp.DecodeBytes(raw, &blockExtraData); err != nil {
log.Debug("error while decoding block extra data", "err", err)
return nil, nil, nil, false
}The TimeNano emission in consensus/bor/bor.go (~line 1147) is gated on Hampi, independently of the Austin gate inside EncodeBlockExtraData:
var timeNano *uint64
if c.config.IsHampi(header.Number) {
value := uint64(header.GetActualTime().UnixNano())
timeNano = &value
}
...
blockExtraDataBytes, err := types.EncodeBlockExtraData(c.chainConfig, header.Number, nil, gasTarget, baseFeeChangeDenom, timeNano)This is a contract change to the pre-Austin wire shape; please confirm the intended fork ordering. On configs where Austin precedes or coincides with Hampi the post-Austin branch is taken and nothing breaks, but dev/test configs enabling Hampi at block 0 hit total loss of validator bytes and base-fee params.
Suggested fix: add the trailing optional field to the mirror struct (zero-risk, since it is rlp:"optional" and last):
type blockExtraDataRawTxDeps struct {
ValidatorBytes []byte
TxDependency rlp.RawValue
GasTarget *uint64 `rlp:"optional"`
BaseFeeChangeDenominator *uint64 `rlp:"optional"`
TimeNano *uint64 `rlp:"optional"`
}Suggested test: in core/types/block_test.go, build a pre-Austin chain config with Hampi active, encode a BlockExtraData with TxDependency, GasTarget, BaseFeeChangeDenominator and TimeNano all set via EncodeBlockExtraData, place the bytes into Header.Extra between vanity and seal, and assert decodeExtraFieldsFast returns ok=true with the expected validator bytes and base-fee params. This fails today with rlp: input list has too many elements.
| return consensus.ErrFutureBlock | ||
| } | ||
| } | ||
| if c.config.IsHampi(header.Number) && header.GetTimeNano(c.chainConfig) == nil { |
There was a problem hiding this comment.
The new Hampi check in verifyHeader RLP-decodes header.Extra a second time per header:
if c.config.IsHampi(header.Number) && header.GetTimeNano(c.chainConfig) == nil {
return errMissingTimeNano
}GetTimeNano calls DecodeBlockExtraData, and a few lines later verifyHeader already decodes the same bytes:
validatorBytes, gasTarget, bfcd := header.GetValidatorBytesAndBaseFeeParams(c.chainConfig)Impact: verifyHeader runs for every imported and gossiped header, so this adds a duplicate decode on a consensus hot path. Worse, DecodeBlockExtraData's pre-Austin branch expands TxDependency into [][]uint64 — exactly the cost the existing fast path was written to avoid:
// blockExtraDataRawTxDeps mirrors BlockExtraData but keeps TxDependency as an
// rlp.RawValue, delaying its decode. Header verification only needs
// ValidatorBytes and the base-fee params, so this avoids expanding a
// potentially large TxDependency into [][]uint64.Hampi is unscheduled on mainnet/amoy (HampiBlock: nil) but is 0 in at least one config, so chains where Hampi activates before Austin pay the full TxDependency expansion per verified header.
Suggested fix: reuse a single decode — e.g. extend GetValidatorBytesAndBaseFeeParams or the fast decoder to also return TimeNano — instead of calling GetTimeNano separately in verifyHeader.
Suggested test: benchmark verifyHeader with a Hampi-enabled, pre-Austin config and a header carrying a large TxDependency payload to quantify the decode cost.
|
|
||
| // TimeNano should match header.GetActualTime().UnixNano() | ||
| // In the non-Rio path, ActualTime is not set so GetActualTime falls back to Time | ||
| expectedTimeNano := uint64(h.GetActualTime().UnixNano()) |
There was a problem hiding this comment.
TestPrepare_HampiTimeNano only asserts self-consistency with GetActualTime on the non-Rio path, where the value is always a whole second:
// TimeNano should match header.GetActualTime().UnixNano()
// In the non-Rio path, ActualTime is not set so GetActualTime falls back to Time
expectedTimeNano := uint64(h.GetActualTime().UnixNano())
require.Equal(t, expectedTimeNano, *timeNano,
"TimeNano should equal header.GetActualTime().UnixNano()")Prepare derives the value from GetActualTime:
var timeNano *uint64
if c.config.IsHampi(header.Number) {
value := uint64(header.GetActualTime().UnixNano())
timeNano = &value
}On the non-Rio branch Prepare sets only header.Time (whole seconds), and GetActualTime falls back to time.Unix(int64(h.Time), 0), so the produced TimeNano always has a zero nanosecond remainder. TestTimeNano_PreservesNanoseconds covers only RLP encode/decode of a hand-built value, not Prepare.
Impact: the PR's stated goal — nanosecond-precision capture through the block production path — has no test that would fail if Prepare produced second-granular timestamps.
Suggested fix: add a Prepare test on the Rio/blockTime path (where header.ActualTime carries sub-second components) asserting the nanosecond remainder is preserved.
Suggested test: configure a Bor with c.blockTime set and IsRio true, run Prepare, and assert uint64(h.ActualTime.UnixNano()) == *h.GetTimeNano(cfg) with a non-zero *timeNano % 1_000_000_000.
2 participants
Summary
Add nanosecond-precision block timestamp capture via new TimeNano field in BlockExtraData. This enables tracking precise block propagation timing beyond the second-precision header.Time field.
Changes:
The Placeholder hardfork name is temporary and will be renamed.
Executed tests
Rollout notes