feat(validator): link validated transactions to signed blocks - #2531
Open
sergerad wants to merge 5 commits into
Open
feat(validator): link validated transactions to signed blocks#2531sergerad wants to merge 5 commits into
sergerad wants to merge 5 commits into
Conversation
This was referenced Aug 31, 2026
sergerad
force-pushed
the
sergerad-validator-tx-block-links
branch
from
August 31, 2026 02:13
47820e4 to
88c1db6
Compare
sergerad
marked this pull request as ready for review
August 31, 2026 02:18
sergerad
requested review from
Mirko-von-Leipzig and
kkovaacs
and removed request for
Mirko-von-Leipzig
August 31, 2026 02:18
sergerad
force-pushed
the
sergerad-validator-tx-block-links
branch
from
August 31, 2026 02:50
88c1db6 to
cbf6c7b
Compare
Comment on lines
+613
to
+615
| // A page size of one row still yields whole blocks: each page overshoots to the end of the | ||
| // block that crosses the limit. A sweep resumes by advancing `block_from` one block past | ||
| // the last block a page returned. |
Collaborator
There was a problem hiding this comment.
Yeah.. this just feels wrong. If a caller asks for 10 items, then send <= 10 items? Or don't allow them to specify at all if you're not going to honor it.
Collaborator
Author
There was a problem hiding this comment.
Replaced with keyset pagination over (block_num, block_tx_index).
Records are written at validation time, before the transaction is in any block, so the schema had no block linkage. Add nullable block_num and block_tx_index columns to validated_transactions plus a partial index, persist each transaction's in-block position in the same database transaction as the signed header, and clear stale links when a header is replaced at the same height. Add the committed-order listing query the administration API will page with in a follow-up.
sergerad
force-pushed
the
sergerad-validator-tx-block-links
branch
from
September 2, 2026 01:05
cbf6c7b to
1fbc55c
Compare
Replace the nullable block_num/block_tx_index columns on validated_transactions with a block_transactions link table. Foreign keys enforce that links reference a stored header and a validated transaction, and deleting a block cascades to its links, so a replacement cannot leave stale links behind. Make block replacement explicit: sign_block passes is_replacement and the writer deletes the replaced block before inserting the new header and its links, instead of REPLACE INTO plus manual link clearing. Simplify the listing to strict keyset pagination over (block_num, block_tx_index): the row limit is honored exactly, and the include_records mode is dropped until an endpoint needs it. Also make the migration comments self-contained and inline the test parameter helpers.
sergerad
force-pushed
the
sergerad-validator-tx-block-links
branch
from
September 2, 2026 01:19
1fbc55c to
6e1547c
Compare
sergerad
commented
Sep 2, 2026
sergerad
commented
Sep 2, 2026
Pull the is_replacement branch out of the database writer: sign_block deletes the replaced block itself and insert_signed_block only inserts, so the writer method has one job. Assert that linking a transaction inserts exactly one row.
Splitting the replacement deletion out of insert_signed_block left the delete and the insert in separate database transactions. Restore atomicity without reintroducing the boolean: replace_signed_block deletes the replaced block and persists its successor in one database transaction, and sign_block branches between it and insert_signed_block.
Every handler repeated map_err(|err| Status::<code>(format!(..., err.as_report()))). Add a StatusResultExt extension trait offering or_internal(context) and or_invalid_argument(context) over any ErrorReport error, and convert the sign_block and submit_proven_transaction call sites. Persistence errors now name the arm that failed (insert vs replace), and the semaphore errors carry their source chain.
sergerad
commented
Sep 2, 2026
| /// Maps a `Result`'s error into a `tonic::Status` carrying `context` and the error's full source | ||
| /// chain, collapsing the `map_err(|err| Status::<code>(err.as_report_context(...)))` boilerplate | ||
| /// every handler repeats. | ||
| trait StatusResultExt<T> { |
Collaborator
Author
There was a problem hiding this comment.
@Mirko-von-Leipzig I don't think this is duplicating #[derive(GrpcError)] because the use case here is mapping at the call site, rather than at the error type. I don't think we want Validator to be redacting internal error contents either.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Part 1 of a 4-PR stack formalizing the validator admin API (#2455).
Private-transaction records are written at validation time, before the transaction is part of any block, so the schema had no way to answer "which block committed this transaction" — which #2455 requires for block-range filtering and pagination.
002adds ablock_transactionslink table keyed on(block_num, block_tx_index). Foreign keys enforce that a link references a stored block header and a transaction validated by this validator, and aUNIQUEconstraint keeps a transaction in at most one block. In-flight and never-committed transactions simply have no link row.SignBlocknow persists each transaction's in-block position in the same database transaction as the signed header (insert_signed_block). Replacement is explicit: the handler passesis_replacement, and the replaced block is deleted first — its links go with it viaON DELETE CASCADE, so transactions dropped by the replacement revert to uncommitted.(block_num, block_tx_index), where the row limit is honored exactly and a caller resumes one position past the last row returned. It is not yet served — the next PR in the stack wires it to the paginated listing endpoint.Legacy rows stay unlinked: a backfill would need per-block transaction ids from the filesystem
BlockStore, which migrations cannot reach today; with the upcoming network resets the legacy set should be small or empty.Stack: #2531 → #2532 → #2533 → #2517
Changelog