Bind service time and enforce request timeouts - #178
Conversation
The SMT leaf value becomes H(txhash, tau) instead of txhash alone, where tau is the reference time of the round the leaf is inserted in. It is equal to IR.Timestamp, so no new source of truth. We bind it. The value is returned explicitly and is persisted with the request and the record. Wire change, not backward compatible: InclusionProofV2 [version, certData, tau, certificateBytes, uc] An SMT built before this change cannot be reused. Refs #176
The certification request carries an exclusive timeout in Unix seconds,
matching Q = (rho, sthash, txhash, tau_Q, u). A request is inserted only
in a round whose reference time satisfies tau < tau_Q; an expired one is
rejected. The timeout constrains predicate validation and inclusion to SMT
only: certification and delivery may occur later.
REQUEST_EXPIRED is reported distinctly from STATE_ID_EXISTS so a client
can distinguish "too late" vs. "already spent".
Wire change, not backward compatible:
CertificationData [version, ownerPredicate, sourceStateHash,
transactionHash, timeout, witness]
Refs #177
With BFT disabled the aggregator certifies rounds through BFTClientStub, and a child shard certifies through RootAggregatorClientStub. Both emitted certificates with no timestamps at all, so a child reading the reference time off the parent certificate saw zero and the service answered every certification request with SERVICE_NOT_READY. TestShardingE2E caught it. Both stubs now carry the two timestamps a live core returns: the input record records the reference time the certified round built its leaves under, and the seal records the time the next round will pin, which is the value the stub already hands the next round. The precomputed proof response fixture declared a version 1 CertificationData holding a timeout, a combination the encoder now rejects; it is a version 2 request.
TestChildPreCollection_CommitmentAfterProofBeforeRoundEnd_ShouldBeInNextRound injected its commitment as soon as block 1 appeared, but finalization and the start of round 2 run concurrently: on that signal alone the commitment can still reach round 2 and land in block 2, which is what the test then fails on. It reproduces on ef141d7, before this branch, in roughly one run in five. Wait for round 2 to be the current round, the precondition the assertion already describes. 8 consecutive runs pass.
CertificationData carried the optional request timeout as two wire versions: version 1 without the field, version 2 with it. The version was then derived from the field rather than read, so it carried no information, and keeping the two consistent needed cross-checks in both marshal directions. Version and element count had to be paired by hand in every decode path. Use one shape instead. ExpiresAt keeps a fixed position and is written as CBOR null when the requester left the deadline to the service, which is how optional elements are already encoded elsewhere in these arrays. Version 2 is now the only accepted version and six the only element count, both checked once. Absence is a nil *uint64 rather than a zero uint64. Zero is a legal instant, so the sentinel could not express "no deadline"; the pointer can. Rename the field to ExpiresAt, since the value is an absolute exclusive deadline in Unix seconds rather than a duration, and the codebase uses "timeout" for transport deadlines elsewhere. Semantics are unchanged. An explicit deadline is used verbatim and is covered by the witness. Without one the service still derives a deadline from consensus reference time; that value stays service metadata, outside the leaf and outside the signature, and no later verifier checks it. Also collapses certDataHashV1 and CertDataHash, which had duplicated the same split.
There was a problem hiding this comment.
Pull request overview
This PR updates the Aggregator’s certification protocol to (1) bind the round reference time into SMT leaf values and (2) support an optional exclusive request deadline (ExpiresAt) with a service-derived default when absent, while also carrying the leaf’s reference time in inclusion proofs so verifiers can reproduce the certified root.
Changes:
- Compute SMT leaf values as
SHA-256(CBOR([txhash, referenceTime]))and persist/servereferenceTimeend-to-end (round → record → inclusion proof → verifier). - Replace version-dependent
CertificationDatatimeout shapes with a single fixed-shape encoding (withExpiresAtencoded as CBORnullwhen absent). - Enforce request expiry using consensus-derived reference time, including fail-fast submit handling and authoritative checks at leaf materialization.
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/sharding_e2e_test.go | Updates integration verification to use LeafValue(txhash, referenceTime) from inclusion proofs. |
| README.md | Documents DEFAULT_REQUEST_TTL, referenceTime in proofs, and expiry/service-not-ready statuses (needs field-name alignment fixes). |
| pkg/api/types.go | Adds ReferenceTime to inclusion proof payload and verifies leaf via LeafValue. |
| pkg/api/README.md | Updates example request to include optional ExpiresAt. |
| pkg/api/leaf_value.go | Introduces shared LeafValue helper implementing H(CBOR([txhash, referenceTime])). |
| pkg/api/leaf_value_test.go | Pins cross-implementation leaf-value test vectors. |
| pkg/api/inclusion_proof_v2_verify_test.go | Updates proof-building tests to use reference-time-bound leaf values. |
| pkg/api/inclusion_cert.go | Updates inclusion cert docs to reference new leaf value derivation. |
| pkg/api/certification_request.go | Collapses CertificationData versions into fixed shape with optional ExpiresAt; updates hashing preimage. |
| pkg/api/certification_request_test.go | Adds CBOR-null round-trip tests and updates hashing compatibility tests for ExpiresAt. |
| pkg/api/certification_request_canonical_test.go | Updates canonical fixtures/markers to match new CertificationData encoding. |
| pkg/api/cbor.go | Adds CborUint helper used in deterministic hashing paths (needs correctness fix for full uint64 range). |
| pkg/api/cbor_tags_test.go | Updates wire-format tests for new array lengths and proof/reference time field. |
| internal/testutil/commitment.go | Adds helpers to generate present/expired ExpiresAt for tests. |
| internal/storage/mongodb/aggregator_record_test.go | Updates record fixtures to include reference time. |
| internal/smt/smt_memory_benchmark_test.go | Updates benchmarks to compute leaf values using reference time. |
| internal/smt/backend/disk_backend_rocksdb_test.go | Updates proof-response fixtures for CertificationData v2 + ExpiresAt. |
| internal/sharding/root_aggregator_client_stub.go | Makes stub parent UC include timestamps so children can pin reference time. |
| internal/service/service.go | Enforces submit-time expiry using consensus reference time and attaches ReferenceTime to served proofs. |
| internal/service/service_test.go | Adds tests for default TTL assignment, service-not-ready, and expiry boundary semantics. |
| internal/round/smt_persistence_integration_test.go | Updates persisted blocks to include reference time. |
| internal/round/round_process_regression_test.go | Passes pinned reference time through round start and leaf materialization paths. |
| internal/round/round_manager.go | Pins round reference time at start; propagates it through round lifecycle and precollector handoff. |
| internal/round/recovery.go | Recomputes/replays leaves from stored records using stored ReferenceTime. |
| internal/round/recovery_test.go | Adds coverage for replay binding reference time into recomputed leaf values. |
| internal/round/precollector.go | Defers leaf materialization until reference time is known; drops expired requests during addBatch. |
| internal/round/precollection_test.go | Updates precollector tests to provide reference time at advance/handoff. |
| internal/round/parent_round_manager.go | Tracks/pins reference time for parent-mode rounds and block creation. |
| internal/round/leaf_add.go | Materializes leaves with pinned reference time; rejects expired requests with explicit error. |
| internal/round/leaf_add_test.go | Adds unit tests for leaf binding and expiry behavior at materialization time. |
| internal/round/finalize_duplicate_test.go | Updates fixtures and leaf computations to use reference-time-bound values. |
| internal/round/factory.go | Extends Manager interface with CurrentReferenceTime() for submit-time expiry checks. |
| internal/round/disk_smt_startup_test.go | Updates disk startup fixtures to use reference-time-bound leaf values and record ReferenceTime. |
| internal/round/disk_ha_failover_integration_rocksdb_test.go | Updates HA failover tests to materialize leaves with reference time. |
| internal/round/disk_bft_integration_rocksdb_test.go | Updates BFT disk-mode tests to set round ReferenceTime and leaf values accordingly. |
| internal/round/batch_processor.go | Drops expired requests during batch processing and ensures blocks record the round’s reference time. |
| internal/proofverify/local.go | Updates local proof verification to require ReferenceTime and use LeafValue. |
| internal/models/certification_request.go | Persists ReferenceTime and EffectiveTimeout; updates request leaf-value derivation. |
| internal/models/certification_request_leafvalue_test.go | Updates tests to assert leaf changes across rounds and doesn’t mutate reference time. |
| internal/models/certification_data.go | Adds optional ExpiresAt to stored certification data. |
| internal/models/block.go | Adds per-block ReferenceTime persisted in BSON and constructors. |
| internal/models/aggregator_record.go | Persists ReferenceTime and EffectiveTimeout on aggregator records. |
| internal/ha/block_syncer.go | Replays leaves from records using LeafValue(txhash, record.ReferenceTime). |
| internal/ha/block_syncer_test.go | Adds test asserting replay binds reference time into leaf values. |
| internal/gateway/docs.go | Updates docs UI text and example CBOR payload to include expiresAt in fixed slot. |
| internal/gateway/docs_test.go | Updates docs example test parsing to include ExpiresAt. |
| internal/config/config.go | Adds DEFAULT_REQUEST_TTL config with validation and default behavior. |
| internal/config/config_test.go | Adds tests for RequestTTL defaulting behavior. |
| internal/bft/client.go | Threads pinned reference time into round starts and input-record timestamps; validates against latest UC seal. |
| internal/bft/client_stub.go | Produces synthetic UCs with timestamps and advances reference time per round. |
| internal/bft/client_stub_test.go | Updates BFT tests/fixtures to include seal timestamps and reference time threading. |
| examples/client/main.go | Updates client example comments for omitted deadline behavior (needs terminology alignment). |
Suppressed comments (1)
README.md:482
- This JSON response example uses the key
timeout, but the API struct field is nowexpiresAt. The example should match the actual JSON field name so copy/paste usage works.
"timeout": 1755003600
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CborUint narrowed its uint64 argument to int to reuse cborTag. That truncates above 2^31 on 32-bit platforms and wraps negative above 2^63 anywhere, where cborTag panics. It feeds hashing paths such as LeafValue and the certification-data preimage, and ExpiresAt is a uint64 taken from the transaction, so the input is not always ours to bound. Encode the full range directly, with a test pinning each width boundary including 2^63 and the maximum. The rename to ExpiresAt left stale references behind: the README's CertificationData example still declared `Timeout uint64` with a `timeout` JSON key and said the transaction hash commits to it, the get_block_records sample still showed `"timeout"`, and comments in internal/models, CertDataHash and the example client still named the old field. The InclusionProofV2 wire comment labelled the slot referenceTimeOrNull. The suffix restated the type rather than naming the field, so the label is referenceTime and the type it carries is written out as `uint | null`.
|
@ristik I pushed two small follow-up fixes for crash recovery and stale proposal cleanup, otherwise looks good -approved. |
|
Took a pass over this. I want to lead with what I didn't find, because I initially thought I had something bigger and it didn't survive checking. I ran four candidate findings through independent verification — durable-proposal resume failing after restart, child-mode cold start pinning reference time 0, a torn read across the precollector handoff, and clients being unable to distinguish an expired request from a pending one. Three were flat wrong and the fourth was overstated. For the record, since the reasoning may be useful:
So the substantive comments are small. I've put fixes for all of them on 1. I verified accept/reject parity against the old implementation across 14 inputs (wrong tag, 5/7 fields, versions 0/1/3, untagged, tagged-non-array, truncated, garbage, empty, nil) with identical decoded values, and kept the interesting cases as a permanent test so the validation surface can't be weakened silently. Note 2. The 1h default request TTL is written twice — once as the 3. 4. Expired requests are dropped with a Worth stating explicitly somewhere in the README, though not a defect: a request with The v1/v2 collapse is a genuine simplification — losing the shadow structs and the two cross-check pairs for −284 lines is the good kind of diff. The Build and vet are clean on Go 1.26; |
Summary
ExpiresAt, in the certification wire formatCertificationDatashape, withExpiresAtwritten as CBOR null when the requester did not supply oneWire format
CertificationDatais one version with one element count:Previously the version was derived from the field (
Timeout != 0 => 2), so itcarried no information, and both marshal directions needed cross-checks to catch
the two disagreeing ("v1 cannot contain a timeout", "v2 requires a non-zero
timeout"). Those are gone, along with the
v1/v2shadow structs and thecase 1:/case 2:arms.certDataHashV1andCertDataHashcollapse to onefunction.
Absence is a nil
*uint64rather than a zerouint64: zero is a legal instant,so the sentinel could not express "no deadline".
Semantics
Unchanged. An explicit deadline is used verbatim and is covered by the witness.
Without one the service still derives a deadline from consensus reference time;
that value stays service metadata, is not inserted into the leaf or the signed
request, and no later verifier checks it.
SERVICE_NOT_READYandREQUEST_EXPIREDare unchanged, and the exclusive boundary is unchanged:tau = deadline - 1is admitted,tau = deadlineis rejected.Validation
All non-Docker packages pass. The remaining failures are the testcontainers
suites (MongoDB, Redis, sharding e2e), which need a Docker daemon.
ValidateCoreDeterministicaccepts CBOR null inside arrays, and fxamackerencodes a nil
*uint64in atoarraystruct as0xf6, both verified before thechange. Cross-implementation bytes are pinned against the TypeScript, Java and
Rust SDKs.
Refs #176
Refs #177