-
Notifications
You must be signed in to change notification settings - Fork 2
fix(api): bind inclusion proofs to the certified shard and network; return referenceTime on block records #182
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.
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
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3083769
fix(service): return referenceTime, expiresAt and finalizedAt on bloc…
MastaP d047422
test(api): pin the exclusive expiry boundary in the shipped verifier
MastaP 91b0d12
docs: add the inclusion proof wire specification
MastaP f25a009
feat(metrics): split accepted requests by deadline origin
MastaP 6f4bd56
docs: correct the inner-node hash rule and bit ordering
MastaP 6c46303
fix: correct verification pseudocode and stop reporting a wrong final…
MastaP f031c4a
docs: correct README against the code it documents
MastaP 17e2ec0
docs: record the yellowpaper divergences instead of documenting them …
MastaP bbcc613
docs: the request timeout is optional per the yellowpaper, not a migr…
MastaP 502a3d4
chore: remove audit scaffolding committed by mistake
MastaP 1eb74e2
chore: remove leaked audit scaffolding; document the predicate CBOR tag
MastaP c767749
fix(api): align inclusion proof verification with SDKs
ristik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| # Inclusion proof wire specification (v2) | ||
|
|
||
| Wire format for `get_inclusion_proof.v2`. Three source comments cite this | ||
| document as normative: `pkg/api/types.go` (`InclusionProofV2`), and | ||
| `pkg/api/inclusion_cert.go` (`InclusionCert`, `ExclusionCert`). | ||
|
|
||
| Corresponds to the Unicity yellowpaper's inclusion proof | ||
| $\pi^{\mathsf{inc}} = (\mathsf{sid}, v, C^{\mathsf{inc}}, UC)$. **The yellowpaper | ||
| is authoritative.** This document describes what the Go implementation actually | ||
| emits, and where the two differ it says so explicitly and names the paper as | ||
| correct -- it does not present an implementation gap as a specification. | ||
|
|
||
| ## CBOR tags | ||
|
|
||
| | Tag | Structure | | ||
| |-----|-----------| | ||
| | 39030 | `CertificationRequest` | | ||
| | 39031 | `CertificationData` | | ||
| | 39032 | `Predicate` | | ||
| | 39033 | `InclusionProofV2` | | ||
|
|
||
| ## RPC response | ||
|
|
||
| The `result` field of `get_inclusion_proof.v2` is a hex-encoded CBOR array: | ||
|
|
||
| ``` | ||
| [blockNumber, #39033([version, certificationDataOrNull, referenceTime, certificateBytes, unicityCertificate])] | ||
| ``` | ||
|
|
||
| `InclusionProofV2` is a tagged 5-element array: | ||
|
|
||
| | Index | Field | Type | Notes | | ||
| |-------|-------|------|-------| | ||
| | 0 | `version` | uint | `1` | | ||
| | 1 | `certificationData` | `#39031([...])` \| null | null ⇒ non-inclusion proof | | ||
| | 2 | `referenceTime` | uint \| null | round reference time τ; null only for non-inclusion | | ||
| | 3 | `certificateBytes` | bstr | `InclusionCert` or `ExclusionCert`, raw (below) | | ||
| | 4 | `unicityCertificate` | raw CBOR | the UC as received from the BFT Core | | ||
|
|
||
| **Discriminator.** `certificationData != null` ⇒ inclusion, and | ||
| `certificateBytes` is an `InclusionCert`. `certificationData == null` ⇒ | ||
| non-inclusion, and `certificateBytes` is an `ExclusionCert`. Non-inclusion is | ||
| neither generated nor verified in Go, and the `ExclusionCert` layout below | ||
| diverges from the yellowpaper -- do not build against it yet. | ||
|
|
||
| ### `CertificationData` | ||
|
|
||
| A tagged 6-element array. The element count never varies with the payload: | ||
|
|
||
| | Index | Field | Type | | ||
| |-------|-------|------| | ||
| | 0 | `version` | uint, `2` | | ||
| | 1 | `ownerPredicate` | `#39032([engine: uint, code: bstr, params: bstr])` | | ||
| | 2 | `sourceStateHash` | bstr(32) | | ||
| | 3 | `transactionHash` | bstr(32) | | ||
| | 4 | `expiresAt` | uint \| null | | ||
| | 5 | `witness` | bstr(65) | | ||
|
|
||
| `ownerPredicate` is **tagged**, not a bare array: `Predicate.MarshalCBOR` emits | ||
| tag 39032 and `Predicate.UnmarshalCBOR` requires it. A predicate with engine 1, | ||
| code `0x01` and params `0x02` encodes as `d99878 83 01 4101 4102`. | ||
|
|
||
| `expiresAt` is the exclusive request deadline τ_Q. It holds its position and is | ||
| written as CBOR `null` when the requester supplied no deadline, so the array | ||
| length never depends on the payload. Absence is distinct from zero: zero is a | ||
| legal instant. Both forms are specified — the yellowpaper's request timeout is | ||
| optional, and `⊥` is written as CBOR null at a fixed position. | ||
|
|
||
| ## Leaf value | ||
|
|
||
| ``` | ||
| v = SHA-256( CBOR([ transactionHash, referenceTime ]) ) | ||
| ``` | ||
|
|
||
| Raw 32 bytes, no algorithm-id prefix. Concretely the preimage is | ||
| `0x82 0x58 0x20 <32-byte transactionHash> <CBOR uint referenceTime>`. | ||
|
|
||
| The leaf value binds the reference time the request was validated under, not the | ||
| transaction hash alone. The tree is append-only, so a leaf can be certified | ||
| afresh against any later root and a later inclusion proof carries a later round's | ||
| `UC.IR.t`. Reference time is therefore a property of the leaf, not of the proof. | ||
|
|
||
| **Do not recover τ from `UC.IR.t`.** Use the `referenceTime` element. They | ||
| coincide only for the proof issued in the leaf's own round. | ||
|
|
||
| ## `InclusionCert` | ||
|
|
||
| Raw binary, no framing: | ||
|
|
||
| ``` | ||
| bitmap[32] || s_1[32] || ... || s_n[32] n = popcount(bitmap) | ||
| ``` | ||
|
|
||
| Siblings are in generation order, root-to-leaf: `s_1` is the sibling at the | ||
| shallowest depth with a bitmap bit set, `s_n` at the deepest. Verification walks | ||
| depths 255..0 and consumes siblings from the end of the slice. | ||
|
|
||
| The certificate carries no root, no key and no value. All three come from | ||
| outside it: | ||
|
|
||
| | Input | Source | | ||
| |-------|--------| | ||
| | key (sid) | the RPC request parameter | | ||
| | value | `SHA-256(CBOR([transactionHash, referenceTime]))` | | ||
| | root | `UC.IR.h` — never a field of the certificate | | ||
|
|
||
| Decoding rejects: fewer than 32 bytes (truncated), a remainder not a multiple of | ||
| 32 (misaligned), and a sibling count disagreeing with the bitmap popcount. | ||
|
|
||
| ## `ExclusionCert` — diverges from the yellowpaper, and is unimplemented | ||
|
|
||
| The Go type encodes: | ||
|
|
||
| ``` | ||
| k_l[32] || h_l[32] || bitmap[32] || s_1[32] || ... || s_n[32] | ||
| ``` | ||
|
|
||
| `appendix-hashtrees.tex` specifies the **opposite order**: | ||
|
|
||
| ``` | ||
| bitmap[32] || s_1[32] || ... || s_n[32] || k'[32] || v' | ||
| ``` | ||
|
|
||
| These are not interchangeable: one logical certificate encodes to two different | ||
| byte strings, and the Go decoder rejects the spec layout with a bitmap/popcount | ||
| mismatch. The spec puts `v'` last so the remainder after the fixed-size terminal | ||
| key is the value; with the fixed 32-byte field leading, a variable-length `v'` is | ||
| structurally unencodable here. The aggregation profile does permit | ||
| `len(v') = 32`, so only the ordering diverges — but `h_l` names the leaf **value** | ||
| `v'`, not a hash of it, which the field name obscures. | ||
|
|
||
| The spec's empty-tree certificate `C^exc_empty` (the empty byte string) is also | ||
| undecodable: `UnmarshalBinary(nil)` returns a truncation error, so a genesis tree | ||
| has no encodable certificate. | ||
|
|
||
| **Nothing generates or verifies these.** `internal/smt` exposes only | ||
| `GetInclusionCert`; `ExclusionCert.Verify` returns `ErrExclusionNotImpl`; and a | ||
| non-inclusion response carries `certificateBytes` as CBOR null (`f6`) rather than | ||
| the spec's empty byte string (`40`). Neither of the two security-critical checks | ||
| the spec names — `k' ≠ k`, and `k[d] = k'[d]` at every junction depth, with the | ||
| region taken from the authenticated terminal key `k'` — exists in this repo. | ||
|
|
||
| This is fail-closed: no forged absence proof is accepted because none is | ||
| accepted. But absence and not-yet-certified are indistinguishable on the wire, | ||
| and this layout should not be treated as frozen until it is reconciled with | ||
| `appendix-hashtrees.tex`. | ||
|
|
||
| ## Hash rules | ||
|
|
||
| - Leaf: `H(0x00 || key || value)` | ||
| - Inner node, two children: `H(0x01 || depth_byte || region(key, depth) || left || right)` | ||
| - Inner node, one child: passthrough, child hash unchanged | ||
|
|
||
| `depth_byte` is the absolute branching depth as a single byte. `region(key, depth)` | ||
| is the 32-byte key prefix addressing the node: the first `depth` bits of the key, | ||
| with every bit at position ≥ `depth` cleared. At depth 0 it is 32 zero bytes; for | ||
| key `0xFFFF…` at depth 12 it is `fff00000…`. | ||
|
|
||
| **The region is not optional.** Omitting it reproduces the correct root only for a | ||
| tree with no binary inner node — that is, a proof with zero siblings. Any proof | ||
| carrying a sibling will verify against a different root. Inner nodes commit to | ||
| their absolute depth *and* to the region addressing them, which is what pins each | ||
| node to its position in the key space. | ||
|
|
||
| ## Bit ordering | ||
|
|
||
| Big-endian (MSB-first) per the yellowpaper: | ||
|
|
||
| ``` | ||
| bit(key, d) = (key[d/8] >> (7 - d%8)) & 1 | ||
| ``` | ||
|
|
||
| So bit 0 is the most significant bit of `key[0]`. Descent at depth `d` goes right | ||
| when `bit(key, d) == 1`, and the sibling supplied at that depth is then the left | ||
| child. | ||
|
|
||
| ## Verification | ||
|
|
||
| `InclusionProofV2.Verify` performs, in order: | ||
|
|
||
| 1. Non-nil proof, request, verifier context and trust base. | ||
| 2. `certificationData != null`, else non-inclusion (unimplemented). | ||
| 3. Request `transactionHash` present, and equal to the proof's. | ||
| 4. The proof and request have equal `expiresAt`, owner predicate, source state | ||
| hash and witness fields. | ||
| 5. `UC.IR.h` extractable and exactly 32 bytes. | ||
| 6. The request `stateId` is exactly the value derived from the certification | ||
| data's owner predicate and source state hash. | ||
| 7. `referenceTime` present. | ||
| 8. If `expiresAt` is present, `referenceTime < expiresAt`. **Exclusive**: a leaf | ||
| created at exactly the deadline is expired. When `expiresAt` is absent this | ||
| check cannot run — the service-assigned deadline is not carried in the proof, | ||
| is not signed, and is not checkable by any later verifier. | ||
| 9. `InclusionCert.Verify(key, LeafValue(txhash, referenceTime), UC.IR.h)`. | ||
| 10. The UC's certified shard is a bit prefix of `stateId`. | ||
| 11. The UC seal network equals the trust-base network. | ||
| 12. Unicity Certificate verification against the expected partition, shard, | ||
| optional shard-configuration hash, and trust base. | ||
|
|
||
| The nil-guard error strings are part of the public contract so reference | ||
| verifiers in other languages can pin them. | ||
|
|
||
| The shard and network checks match the JavaScript, Java and Rust state-transition | ||
| SDK verifiers. `ExpectedShardID` remains an additional caller policy check for | ||
| backward compatibility; it does not replace deriving the state ID or checking | ||
| that the UC's certified shard contains it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| package service | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "math/big" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/prometheus/client_golang/prometheus/testutil" | ||
| "github.com/stretchr/testify/require" | ||
| bfttypes "github.com/unicitynetwork/bft-go-base/types" | ||
|
|
||
| "github.com/unicitynetwork/aggregator-go/internal/config" | ||
| "github.com/unicitynetwork/aggregator-go/internal/logger" | ||
| "github.com/unicitynetwork/aggregator-go/internal/metrics" | ||
| "github.com/unicitynetwork/aggregator-go/internal/models" | ||
| "github.com/unicitynetwork/aggregator-go/internal/signing" | ||
| "github.com/unicitynetwork/aggregator-go/pkg/api" | ||
| ) | ||
|
|
||
| // The get_block_records response is documented in README.md. A consumer needs | ||
| // referenceTime to rebuild the certified leaf value and expiresAt to check the | ||
| // request deadline, so silently dropping either makes the record unverifiable. | ||
| // This pins the exact key set the endpoint emits. | ||
| func TestBlockRecordWireShape(t *testing.T) { | ||
| expiresAt := uint64(1755003600) | ||
| record := &models.AggregatorRecord{ | ||
| StateID: api.RequireNewImprintV2("c7aa6962316c0eeb1469dc3d7793e39e140c005e6eea0e188dcc73035d765937"), | ||
| CertificationData: models.CertificationData{ | ||
| OwnerPredicate: api.Predicate{Engine: 1, Code: []byte{0x01}, Params: []byte{0x02, 0x03}}, | ||
| SourceStateHash: api.RequireNewImprintV2("539cb40d7450fa842ac13f4ea50a17e56c5b1ee544257d46b6ec8bb48a63e647"), | ||
| TransactionHash: api.RequireNewImprintV2("c5f9a1f02e6475c599449250bb741b49bd8858afe8a42059ac1522bff47c6297"), | ||
| ExpiresAt: &expiresAt, | ||
| Witness: []byte{0x04, 0x05}, | ||
| }, | ||
| ReferenceTime: 1755000000, | ||
| BlockNumber: api.NewBigInt(big.NewInt(123)), | ||
| LeafIndex: api.NewBigInt(big.NewInt(0)), | ||
| CreatedAt: api.NewTimestamp(time.UnixMilli(1734435600000).UTC()), | ||
| } | ||
|
|
||
| encoded, err := json.Marshal(modelToAPIAggregatorRecord(record)) | ||
| require.NoError(t, err) | ||
|
|
||
| var decoded map[string]any | ||
| require.NoError(t, json.Unmarshal(encoded, &decoded)) | ||
|
|
||
| require.ElementsMatch(t, | ||
| []string{"stateId", "certificationData", "referenceTime", "blockNumber", "leafIndex", "createdAt"}, | ||
| keysOf(decoded), | ||
| "get_block_records record keys changed; update README.md to match") | ||
|
|
||
| certData, ok := decoded["certificationData"].(map[string]any) | ||
| require.True(t, ok) | ||
| require.ElementsMatch(t, | ||
| []string{"version", "ownerPredicate", "sourceStateHash", "transactionHash", "expiresAt", "witness"}, | ||
| keysOf(certData), | ||
| "certificationData keys changed; update README.md to match") | ||
|
|
||
| require.EqualValues(t, 1755000000, decoded["referenceTime"]) | ||
| require.EqualValues(t, 1755003600, certData["expiresAt"]) | ||
| require.EqualValues(t, api.CertificationDataVersion, certData["version"]) | ||
| // finalizedAt is deliberately not emitted: nothing persists a finalization | ||
| // timestamp, and the block's CreatedAt is proposal time. | ||
| require.NotContains(t, decoded, "finalizedAt") | ||
|
|
||
| // An absent deadline stays absent rather than becoming zero: the service | ||
| // assigns its own, but that value is not part of the certified record. | ||
| record.CertificationData.ExpiresAt = nil | ||
| encoded, err = json.Marshal(modelToAPIAggregatorRecord(record)) | ||
| require.NoError(t, err) | ||
| require.NoError(t, json.Unmarshal(encoded, &decoded)) | ||
| certData, ok = decoded["certificationData"].(map[string]any) | ||
| require.True(t, ok) | ||
| require.Nil(t, certData["expiresAt"]) | ||
| } | ||
|
|
||
| func keysOf(m map[string]any) []string { | ||
| out := make([]string, 0, len(m)) | ||
| for k := range m { | ||
| out = append(out, k) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| // The deadline-origin counter reports the share of traffic relying on the | ||
| // service-assigned deadline, so it must count only requests that were actually | ||
| // accepted. An expired request is rejected and must not be counted. | ||
| func TestDeadlineOriginCountsOnlyAcceptedRequests(t *testing.T) { | ||
| ctx := context.Background() | ||
| log, err := logger.New("error", "text", "stdout", false) | ||
| require.NoError(t, err) | ||
|
|
||
| const referenceTime uint64 = 1755000000 | ||
| read := func(origin string) float64 { | ||
| return testutil.ToFloat64(metrics.CertificationRequestsByDeadline.WithLabelValues(origin)) | ||
| } | ||
|
|
||
| newService := func(queue *recordingCommitmentQueue) *AggregatorService { | ||
| shardingCfg := config.ShardingConfig{Mode: config.ShardingModeBFTShard} | ||
| return &AggregatorService{ | ||
| config: &config.Config{ | ||
| Processing: config.ProcessingConfig{SkipDuplicateCheck: true, DefaultRequestTTL: time.Hour}, | ||
| Sharding: shardingCfg, | ||
| }, | ||
| logger: log, | ||
| commitmentQueue: queue, | ||
| roundManager: &stubRoundManager{referenceTime: referenceTime}, | ||
| certificationRequestValidator: signing.NewCertificationRequestValidator(shardingCfg, bfttypes.ShardID{}), | ||
| } | ||
| } | ||
|
|
||
| // An accepted request with no deadline counts as service_assigned. | ||
| before := read("service_assigned") | ||
| queue := &recordingCommitmentQueue{} | ||
| accepted := createTestCertificationRequests(t, 1)[0] | ||
| accepted.CertificationData.ExpiresAt = nil | ||
| resp, err := newService(queue).CertificationRequest(ctx, accepted) | ||
| require.NoError(t, err) | ||
| require.Equal(t, "SUCCESS", resp.Status) | ||
| require.Len(t, queue.stored, 1) | ||
| require.Equal(t, before+1, read("service_assigned")) | ||
|
|
||
| // An expired request is rejected and must not be counted at all. | ||
| beforeExplicit := read("explicit") | ||
| beforeAssigned := read("service_assigned") | ||
| queue = &recordingCommitmentQueue{} | ||
| expired := createTestCertificationRequests(t, 1)[0] | ||
| expired.CertificationData.ExpiresAt = api.Uint64Ptr(referenceTime) | ||
| resp, err = newService(queue).CertificationRequest(ctx, expired) | ||
| require.NoError(t, err) | ||
| require.Equal(t, api.CertificationStatusRequestExpired, resp.Status) | ||
| require.Empty(t, queue.stored) | ||
| require.Equal(t, beforeExplicit, read("explicit"), "a rejected request must not be counted") | ||
| require.Equal(t, beforeAssigned, read("service_assigned")) | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When any accepted request omits its deadline, this Prometheus counter becomes nonzero and can never “reach zero” as the migration condition above requires; conversely, a process restart resets it to zero even if unmigrated clients are still active. This makes the advertised signal unsafe for deciding when to reject missing deadlines. Track the absence rate over an explicit observation window (or represent an actual outstanding population) rather than treating the raw cumulative counter as a backlog.
Useful? React with 👍 / 👎.