Repository navigation
Conversation
Fees stay in log space. The batch-scoped nonce with a sequencer-filled look-back would make replay state prunable and let the fee move to the batch; record it with its blocking gap (after a recovery rollback, a re-signed op does not exclude the original), and unblock Track 5 pending a sync with Bart.
Four server helpers turned a PermissionDenied bind into an eprintln and None, and 42 callers returned early, so a sandbox that denies bind reported the HTTP, WS and snapshot suites green without running them. The helpers now panic on any bind error and return the runtime.
A manual dispatch checked out the dispatching branch but named every artifact, image and the GitHub release after inputs.tag, and recorded GITHUB_SHA as the commit; a missing tag was then created at the default branch's head. A push now builds the pushed commit, and a dispatch builds the named tag by its fully qualified ref, so a branch or SHA input cannot stand in and a missing tag fails. Manifests and the image record the checked-out commit. Tag pushes were never marked pre-release, so each alpha became the latest release; SemVer pre-release tags (with '-') now are.
download-deps verified the kernel against a .sha512 downloaded from the same release, which catches transfer corruption but not a replaced asset. The kernel is part of the canonical template hash, so it now must match an in-tree pin, like every other downloaded input.
- The 429 test raced a spawned request for the lane queue's one slot and could hang if it lost; the server now starts with the queue full. - The transient-poster-error test asserted after a fixed 250 ms and failed under load; it now waits for the first submission. - Two e2e scenarios slept 7 s for the submitter's tick; they now wait until the submitter's tx is in Anvil's mempool.
- init deleted every checkpoint when config.json was missing, including an agreed head; it now removes only an interrupted init's bootstrap checkpoint and refuses otherwise. - Idempotent init now validates an existing config.json the way tick does, instead of reporting success and failing the next tick. - clear created a missing state directory and then reported that no divergence was latched; it now refuses a missing directory. config.json joins the backup list, since init no longer regenerates it over an agreed head.
With 0, the submitter's idle and transient arm slept for zero and re-ran its frontier and nonce RPCs back to back. The flag now takes the same range(1..) as the other poll intervals.
run and flush-mempool created the data dir and its lock file before finding no database, then exited 30 with a message that named no path. Only setup creates the directory now; run and flush refuse a missing one before taking the lock, and SetupNotComplete names the data dir. An unreadable or non-directory path stays an I/O error (exit 1).
TipAlreadyOpen was classified retry (exit 20), so the next boot would find the Tip and admit. Under the process lock only another writer or an inspection bug can produce it, and admitting would hide that; it now refuses (exit 30), like its postcondition sibling TipMissingAfterOpen. Aging-driven StaleDecision still retries.
The lane checks the shutdown flag only at the top of its loop. When graceful HTTP shutdown dropped the queue's senders after that check, the next turn returned ChannelClosed, which the drain treats as an error, so a clean SIGTERM could exit 1. ChannelClosed with shutdown requested now returns Ok. A lane test drives the race deterministically, and the in-crate server tests assert a clean stop instead of tolerating ChannelClosed.
fold_replay's comment claimed everything <= C was enforced, but only the post-drain empty-fridge check caught out-of-range inputs, and a replay batch above C leaves nothing in the fridge, so it was folded silently. Seeds and replay inputs now assert their block against the stop block; the fridge check stays as a backstop. The only caller already bounds its inputs. Also drops a review codename from a comment.
The header reserves NOT_FOUND and INVALID_DUMP for dump lifecycle calls, but the adapter mapped them to I/O errors from validate, execute and drain too, where the lane treats I/O as retryable: an engine bug became a restart loop instead of a terminal stop. The adapter now maps them to Internal outside from_dump/create_dump, and the reference engine reports them only from those two calls.
Every other client call is bounded, but subscribe() awaited the WS handshake with no deadline and hung on a stalled server; the harness wrapped it in its own timeout. It now fails with HandshakeTimeout after the configured request timeout.
…ding - AGENTS.md told every app to implement all four rejections; apps implement InvalidNonce and InsufficientFeeBalance, and the shared boundary alone produces InvalidMaxFee and NonceExhausted. - The scheduler spec and README now say the canonical machine's compiled sequencer_address must equal the pinned submitter, which nothing checks at runtime. - cockroach.md said the watermark is unavailable because the database is lost; recovery reads the fresh directory even when the old one survives. - A lifecycle comment claimed the completion rule had production callers only on the run side.
The Track 3 plan still had Bart integrating scheduler replay over the historical-L1 API. His client treats cockroach recoveries as out of scope, and a planned shared recovery tool will produce client startup state, so the cross-rebuild projection path receives bug fixes only.
The optimistic model describes a design that was not adopted. It stays as a dev analysis tool behind check-optimistic, out of check-all.
- The testsi cycle-target entry is fixed in the vendored copy (relative target since 1283765); delete it. - The RPC-URL entry also covers transport errors in the reader, poster, flusher and fee oracle, and the watchdog's persisted state. - The SDK entry narrows to HTTPS for ingress; WSS waits for the new sync. - The admission entry names where setup reaches the completion rule. - The fee-observation entry records the per-poll stamp's purpose and cost.
The SDK, the harness and the sequencer's tests pinned reqwest 0.12, so the build compiled two reqwest stacks. reqwest 0.13 puts RequestBuilder::query behind a feature, which the SDK now enables.
testsi pinned thiserror 1 and two members pinned toml 0.8, each keeping a second copy of those stacks in the build; both now use the workspace version (toml moves into [workspace.dependencies]). The info.toml and checkpoint.toml documents are flat integer tables, which toml 1.x reads and writes the same way. The sequencer no longer declares alloy-network-primitives, which nothing used.
The client rejected every https:// endpoint. It now accepts http:// and https://, using reqwest's rustls feature, the TLS stack alloy already builds, with the platform trust store. The WS feed stays plain ws://, so an https client cannot subscribe; the subscribe() doc says so. A test pins that an https request opens with a TLS ClientHello; certificate rejection is recorded as a verification gap.
Thirteen error variants wrapped a #[source] but left it out of their
Display, and every log and abort sink prints Display only, so an exit
read 'snapshot at batch close failed' with no reason. They now end with
': {source}', the convention CommandError and BootstrapError already
use. Three feed and history messages also named the wrong operation.
The variant names what prepare() actually reads.
One helper names the path an I/O error happened at and keeps its kind.
The data-dir and dumps-dir creates, the database probes, the process lock and the orphan sweep surfaced bare OS errors such as 'Not a directory (os error 20)', which name no file. They now say which path failed and keep the error kind, so exit codes do not change. The lock's I/O variant carries its path.
…shes The flusher watched each submitted no-op's hash for inclusion. When the original batch tx won a slot instead, the no-op never landed and the pass waited out its full 10-block timeout before re-checking. It now waits, under the same timeout, until the account's Latest nonce reaches the flush end, whichever tx consumed each slot. Errors still propagate; the outer loop is unchanged.
Every failed command logged its exit line at ERROR, including expected recovery (10) and retryable exits (20), which breaks ERROR-based alerting. Terminal and unclassified exits keep ERROR; the others log at WARN. The line now names the verdict and the supervisor action, and the SIGABRT abort line says it is terminal.
The reader and the submitter logged every retried provider failure at ERROR, once per poll during an L1 outage. They now log at WARN, and the provider error names the failing call (eth_chainId, eth_sendRawTransaction nonce=N, ...). A watermark write failure on the submitter path logs as such instead of as a provider error. A test pins that the mempool-conflict classifier still matches labelled messages.
The CANONICAL DIVERGENCE line named the nonce and input index only; an
operator then had to join tables to find the offending transaction and
compare the two sides. The page query now reads the transaction hash,
and the line carries it, the landed payload's hash and ours ("none" for
a foreign landing at a nonce we never sealed).
AGENTS.md restated contracts that other documents own: the scheduler spec, the recovery designs, the application contract, the invariant register, the README API and Running sections, and the threat model. Each restatement was a second copy to keep in sync. Sections now state the idea in a few sentences and link the owner; Key Concepts and Domain Truths keep the entries a newcomer needs and drop those with another home. Ordering and Storage goes: its single-lane rule is an Ask First item, its pointers are Reading Routes rows, and its storage caution joins the Always bullet on the register. The guide falls from 478 lines and 5,110 words to 347 and about 3,570. The dedicated-submitter rule moves to README Running, where an operator configuring setup reads it, in the same commit that drops it here, so it never lacks a home. Headings with inbound links stay: Application Trait Contract, HTTP Endpoints, Testing Guidance, Shell and Commands, Invariants, and Reading Routes. The bold label InputBox payload classification stays for batch.rs's citation. Workspace gains the watchdog, and the threat-model links point at its RPC endpoint and fee oracle sections instead of the actor table.
… them The overview restated how the content-identity check runs, when it fires, and what it does not prove; I9 and I15 own those facts. The paragraph keeps what an operator needs from the overview: what the check compares, that a mismatch stops the runtime until cockroach recovery, and that it does not replace the watchdog. The bounds stay behind the existing link.
As written, the setup example fails on chain 31337: an unknown chain needs an explicit fee-oracle source, so the example sets FIXED_LOG_GAS_PRICE=0. It also used Anvil account 0, the contract deployer, as the submitter; the devnet compiles account 9 into the canonical machine, so the example uses account 9 and its key and matches the dedicated-submitter rule beside it. The optional-variable line listed defaults that config.rs owns and missed the fee-oracle, recovery, batch-open and poll variables. It becomes a names-only list grouped by subcommand, with --help as the home of defaults and validation. The required lines cover flush-mempool and the key-file alternative. The closing line about internal runtime constants narrated history and named polling intervals that are configurable, so it goes.
The Storage Model entry for batch_policy had grown into an essay on oracle bootstrap, refresh, host float pricing, the tenfold slack, the negative gas-price exponent, and fee units. The schema header beside the table owns the formula and those encodings, the invariant register's writer table owns the first price write, and the threat model's fee oracle section owns how the oracle prices and refreshes. The entry keeps what the table is, links those owners, and keeps the batch-close rule, which has no other home.
The scheduler spec's user-op signatures section owns the domain: its type string, name, version, the absent salt, and where chainId and verifyingContract come from. Running keeps the operator's part, which variables set the two deployment fields, and links the spec for the rest. The API's GET /domain example stays, since the API owns it.
"Initially" and "startup no longer stamps" described the lifecycle by what it used to be; the text states what holds. The orphan-sweep paragraph restated startup_hygiene.rs line by line, including its error classification; it keeps the reason the sweep compares device and inode instead of paths, and that it resolves every retained artifact before deleting, and links the code that owns the rest. The wallet format drops the name of the test that pins entry order: the ordering rule is the contract, and the test can move without the doc.
…shots index README's Project Layout and Further Reading repeated AGENTS.md's architecture map and Reading Routes and had already drifted from them: the layout missed canonical-app, canonical-test and the SDKs. README keeps a short paragraph on how an application composes the library, and links the architecture map and Reading Routes. The watchdog image names live in the operator deployment guide. Local Test Prerequisites goes: Development links the testing guidance, which owns the Anvil and libslirp requirements. docs/snapshots/README.md was an index whose only content of its own was the three-boundary paragraph. That paragraph moves to the top of the lifecycle doc, without the pointer to snapshot-plus-replay bootstrap, so new readers are not sent to the frozen bootstrap flow. The index is deleted with its one inbound link.
…ale once The worker header walked through each tick step by step and restated the outer loop's cadence, which TickOutcome's variants already own; four lines now say what a tick does and why cancellation is safe. The poster explained the pre-send chain-id recheck twice, in the config field and at the check. The field now points at the check, and the check names the assumption it backs up, the threat model's one-consistent-node rule, and what it still prevents: a swapped chain's wallet nonce reaching the durable watermark. It costs one call per productive tick; revisit when a fallback RPC tier lands.
advance_expected_batch_nonce sat beside scheduler_accepts as if it were the off-chain mirror of the scheduler's nonce rule, but its only caller is the submitter's decide_submit_start, which folds latest (not safe) events and applies no staleness. It is a resubmission heuristic: a stale batch does not consume its nonce on chain, and the gold frontier advances inline beside its content check. The loop now lives in decide_submit_start, whose doc says what it is, and leaves the guest crate. Pure move, no behavior change: the same equality fold and the same overflow panic. The core table tests move into the worker's tests as the rows its four existing tests did not cover, plus the overflow case. The worker test comment drops its pointer to the deleted doc and its false "table-tested below".
batch.rs carried a 33-line essay on batch sizing and fees that the schema header beside batch_policy already owns, and two of its claims were false: the knobs are not hot-swapped at runtime (the table has no runtime writer), and the size CHECK compares against log_max_batch_bytes, not a const_max_batch_bytes. Two lines now name the owner. The InputBox cost model (gas ~ base_gas + delta x payload bytes; alpha x delta x batch_size_target = base_gas) lives in that schema header's "Batch sizing (view)" block in sequencer/src/storage/migrations/0001_schema.sql, placed there by the storage commit "home the InputBox cost model with batch_policy". The log-space encoding stays with crate::fee, and the frame-fee lifetime with frames.fee. The classification note cited an AGENTS.md label; it now cites the protocol spec section that owns the one-stream classification.
Module headers stay within four lines and name their owners instead of restating them. protocol.rs says what the module holds and that its acceptance predicate must agree with the canonical fold (I1); the batch-submitter-as-parameter rationale stays once, on the ProtocolTiming type doc, and leaves scheduler_accepts. fee.rs keeps the encoding, the units it covers and the bit-for-bit porting rule, with fee_to_linear as the conversion owner. SUGGESTED_MAX_FEE_SLACK says what it is and how it differs from batch_policy.log_slack; the FeeResponse type doc already says why wallets get the slack. The wallet snapshot header names format.md as the layout owner and drops an intra-doc link that did not resolve. The canonical-app shell and the golden-case header lose their restated detail; consensus_vectors.rs owns "never regenerated".
Three scheduler comments told how the code got here: inspect_state's visibility "not pub(super)" because the harness is "now" a separate crate, and two tests' notes on what used to be tested only partly. The comments keep the present-tense reason and the property each test pins. Folded into the guest batch so the template hash moves once.
Four doc comments, the schema comment on safe_accepted_batches, and invariants I3 and I12 named Storage::append_safe_inputs, which became a test seed in storage::test_helpers: rustdoc could not resolve the links, and all of them named the wrong writer. They now name append_ingested_safe_inputs_with_timestamp, which holds the monotonicity asserts and maintains safe_accepted_batches in production. The schema comment says a parentless root carries the batch-tree anchor's nonce, 0 at genesis, as its trigger enforces. register.md links the oracle outage policy at its new threat-model subsection, and cartesi-machine.md drops a 'no longer exists' history note.
W4's guest commits trimmed comments in sequencer-core, app-core and canonical-app and moved advance_expected_batch_nonce into the submitter, so the template hash moved again. The cycle count to the first yield is unchanged, as comment edits only shift the line numbers rustc embeds in panic locations. Both moves ship in the same next image, so the existing record gains a W4 row and new final values instead of a second record: devnet 0xa0c7296d..., Sepolia 0x410cc0e1..., cold builds, consensus vectors byte-identical, canonical-test-guest 15/15.
W4's progress line, what it delivered with the AGENTS.md before and after counts, the porter spec, the threat-model restructure, the header cap, the nonce-scan move and the README fixes. It also records what was skipped as already fixed or deferred to the recovery tool, the FS1 and B1 rows as decided and documented, N25 as done, and two items under Not doing: a CI check for the size caps, and bounding archive downloads per connection.
GCdePaula
changed the title
GCdePaula
marked this pull request as ready for review
October 15, 2026 10:17
Pin the v0.22.0 emulator and the kernel its dependencies.lock pairs it with, cartesi/linux v6.18.56-ctsi-1. machine-linux-image stops at v0.21.0, so the kernel now downloads from cartesi/linux, its release pin is renamed CARTESI_LINUX_RELEASE, and the pins check ties the kernel filename to that tag. Guest tools stay at 0.18.0. cartesi/linux tags name the kernel version, so no in-tree check can pair the kernel with the emulator any more, as the old release-equals- version check did. verify-toolchain-pins.sh --upstream, run before each release, compares the kernel and guest tools tags with the emulator's dependencies.lock and the kernel SHA-512 with the one its release publishes. v0.22 refuses machines stored by v0.21 (config archive version 8, marchid 22), so every image and watchdog checkpoint is regenerated. The template hash moves even for identical inputs: the machine identifiers and the microarchitecture's state are part of the root hash. testsi moves to the v0.22 Rust bindings at Dave's v3.0.0-alpha.6 tag, which pass the u32 cmio length the v0.22 header takes. The watchdog's CLI oracle names the renamed input_file_index_* keys, since the v0.22 CLI rejects unknown --cmio-advance-state keys.
…dget v0.22 reads a stopped machine's outcome in the order halt, manual yield, then exhausted budget, so an RX_ACCEPTED or RX_REJECTED yield on the budget's last cycle is an accept or a reject for the CLI and Dave. The watchdog checked the budget first and would have latched a dead canonical machine there instead, and refused a stored head in that state. Check the flags first, in the emulator's order. Reaching the budget end to end takes about 2^48 cycles, so a fake cartesi module pins the order.
v0.22 makes a cmio response the machine cannot take a silent no-op: with no yield pending, an advance at any yield but RX_ACCEPTED, or data larger than the RX buffer. v0.21 refused those sends, and the README's case for running in-process leaned on that refusal. The watchdog never sends in those states, but it now asserts after each send that iflags.Y cleared, which only delivery does, so a broken precondition fails loud instead of reading the old yield as the input's outcome.
v0.22 makes an advance-state response the machine cannot take a silent no-op. testsi already checks for an rx_accepted yield before sending, but an input larger than the 2 MiB RX buffer would be dropped: run would return at the same yield, and advance_state would report an accept with no outputs. Check after sending that iflags.Y cleared, which only delivery does, and fail with InputNotDelivered otherwise. The comments that named v0.21 now state the rules they rely on.
The page holds for the pinned emulator and asks for a re-check on every bump. Against v0.22.0 three statements were false: a response the machine cannot take is a silent no-op, not a refusal; the CLI asserts an RX_ACCEPTED yield before an inspect query; and --no-revert aborts at the first rejected input. Correct those, and add what the page left out: how a version mismatch is detected, why an emulator bump alone moves the template hash, the outcome order at the budget limit, the SHARING_ALL refusals of read-only and hard-linked files, the renamed --cmio-advance-state keys, and --assert-rolling-template's outputs-root check. The kernel now comes from cartesi/linux, and the storage measurements are labeled as v0.21.0's.
v0.22 refuses to map a file for writing in place when it has no write permission, and clone_stored hard-links read-only files, so a bootstrap machine made read-only fails at the first tick: as root with "is read-only and cannot be shared for writing", otherwise at open with "Permission denied". v0.22 also loads nothing an older release stored, and a clone reports that as an internal error, not as the documented operator one. Say to leave the bootstrap machine's permissions as --store sets them (the PMAs file alone read-only), to keep a release per Cartesi Machine version for replay and evidence, and to start a new deployment's watchdog in a new state directory, keeping the old one intact. List the messages in troubleshooting, and tell developers that recipes reuse images stored by another emulator version.
The record keeps one ledger from the deployed baseline to the next image, since every move since then ships in that image. It gains the Cartesi Machine v0.22.0 and kernel 6.18.56-ctsi-1 row and new final values: devnet 0xcd299178... (53967473 cycles), Sepolia 0xeaf87115..., both from two agreeing cold builds. The devnet dapp and rootfs equal W4's, so the template moved through the emulator and kernel alone; the Sepolia dapp also carries the dedicated sequencer address from aa0fc0f, which was not measured on its own. The review record's progress line names the bump.
GCdePaula
force-pushed
the
claude/repo-code-review-103cc5
branch
from
October 20, 2026 15:03
aec6530 to
ee79a8d
Compare
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
2 participants
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.
What
Waves W1–W4 of the 2026-10-13 whole-repo review, plus the input-reader review branch (
claude/input-reader-review-fda7ad, adapted), as one stack.Plan, decisions, and what each wave landed: docs/review/2026-10-13-whole-repo-review.md.
test_support, storage fixtures instorage::test_helpers, the benchmark crate trimmed.safe_inputs.aa0fc0f): re-signed with author kept, becausemainrequires signed commits; same tree asaec6530.v6.18.56-ctsi-1); Dave bindings at tagv3.0.0-alpha.6. The watchdog and testsi now check every cmio response reaches the machine (v0.22 drops a refused one silently), and the watchdog ranks halt and manual yield above an exhausted budget. Facts re-checked in docs/cartesi-machine.md.Risk and compatibility
tests/fixtures/consensus_vectors_v1.json. Hashes: guest image record. The next release's notes must state the new hash.replayold checkpoints and evidence (upgrades).rust-srconly on a later run, so the hash depended on build-cache history.just canonical cross-toolchainnow runs before every guest build.tx_nonce→wallet_nonce; a 3xx at setup's chain-id check exits 30; an undecodable batch from our own key abortsrun(terminal); genesis-dump PermissionDenied exits 1; L1 error text is one line, capped, without the RPC URL.scripts/verify-action-pins.sh; runs on PRs and pushes tomain/v*; cargo-deny; runner pinned toubuntu-24.04.ghreplacesaction-gh-release,download-artifact8 /upload-artifact7,build-push-action7. Worth a rehearsal with a throwaway tag on a fork.Verification
cargo test --workspace,clippy -D warningson every W3b commit, fmt, Lua tests and luacheck.just canonical-test-guest, including the golden consensus cases, on cold-built images; two cold v0.22 builds agree.