Repository navigation
Fix Dave QA findings - #303
Merged
Merged
Conversation
stephenctw
force-pushed
the
fix/qa-report-findings
branch
from
October 20, 2026 13:07
4dc351d to
7419c8c
Compare
Member
Author
|
@jplgarcia I can't add you as a reviewer, but your feedback is very valuable to us. |
stephenctw
force-pushed
the
fix/qa-report-findings
branch
from
October 22, 2026 14:54
7419c8c to
e4ba98d
Compare
guidanoli
reviewed
Oct 23, 2026
stephenctw
force-pushed
the
fix/qa-report-findings
branch
from
October 24, 2026 02:33
91c58f1 to
a6748d4
Compare
stephenctw
changed the title
stephenctw
force-pushed
the
fix/qa-report-findings
branch
from
October 26, 2026 13:27
a6748d4 to
8b68486
Compare
stephenctw
changed the title
Uncapped acceptance could finalize inputs the state transition cannot feed. Cap construction and acceptance at the Machine input span, preserving excess inputs for later epochs and finalizing only processed inputs. Check the emitted EpochSealed bounds as well as storage. Split pending input windows at the same boundary in Rust, and reject seals past that span with the capped-consensus requirement. Lua selects inputs by sealed bounds and refuses an epoch past the span; it does not split. Go rollover must select inputs by sealed index bounds across block ranges. Keep the single delay ledger and reference price explicit: capped work can still drain C, while spill-epoch queue waits do not. Re-derive the price below the throughput threshold after Linux qualification. Addresses move. Earlier uncapped deployments are unsupported and stall at the first seal past the span. Addresses PRT-1 and SLN-42.
A caller could use newDaveApp with a victim's application parameters and attacker-chosen consensus settings to occupy the predicted app address. Bind the salt to every consensus setting in both deploy and predict. Retain the shared ApplicationFactory. Its direct-deployment squatting path remains open pending the reserved API in cartesi/rollups-contracts#561; do not send value before DaveAppCreated. If no release ships it before mainnet funds or the audit scope freeze, choose between waiting and creating Application directly. Test different settings, the identical pair and fuzzed address binding. Predicted addresses move. Partially addresses CTR-1.
The underlying terminal-epoch, HTIF-read and startup-sweep fixes are already on main. Pin their behavior without claiming new fixes: defend a halted guest through a leaf proof across restarts, stage an oversized yield through settlement, and assert that initial seeding publishes its boundary after sweeping stale staging. Share the tiny guest builder with the engine tests and use one nonstandard-yield world. Keep the existing split-any-error RPC policy under provider range codes, including Alchemy's -32600. Covers SLN-3, SEAM-2, SLN-59 and the SLN-56 regression.
Main kept defending through foreclosure but never stopped. Read foreclosure once the dispute has a result, suppress settlement while visible, and exit 30 only after defense and bond recovery are complete and foreclosure is finalized. Restarting resumes the same obligations. Keep Hero and recovery active. Enable the harness guardian and test foreclosure during a live dispute and after staging. Recover the bond before foreclosure in the latter case, so latest-only foreclosure cannot be mistaken for a finalized exit. Pin the exit across restart. Addresses the remaining SLN-4 behavior.
Waiting for the terminal epoch's staging error can delay the guardian's first page by a root allowance while users keep sending deposits. Detect the transition from awaiting input to a terminal state and emit one ERROR after its batch commits. Retried or abandoned batches do not page twice; later inputs in the absorbing state do not page again. Continue defending the true state and ask the guardian to foreclose. The real-runner test pins the first awaiting-to-terminal transition the guard reads, including the absorbing state on a later input; it does not capture the page or test retries. An open batch can still wait for its snapshot gap or the seal. Relates to SLN-3.
An omitted pairing can leave the event-derived candidate idle while its match clock runs. Detect that contradiction only under active matches, where waiting can lose defense, using the standing already read at the same head rather than another RPC. Retain the active candidate in the projection and propagate a typed mismatch so the reader discards and refolds its cache. Logs remain a trusted provider property; this is defense in depth for SLN-60.
Release-list discovery reported the published pinned just version missing and stopped CI before tests. Fetch the exact Linux assets for both host architectures and verify their checksums before exposing the executable. Keep the version pinned: floating latest silently changes CI's toolchain. The native installer and both archive checksums were verified locally.
stephenctw
force-pushed
the
fix/qa-report-findings
branch
from
October 27, 2026 12:00
8b68486 to
2e48dcb
Compare
GCdePaula
approved these changes
Oct 27, 2026
GCdePaula
left a comment
GCdePaula
left a comment
Member
There was a problem hiding this comment.
LGTM!
Let's wait for @guidanoli to give a final review too on the smart contracts.
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.
DaveConsensus could seal more inputs than its computation can represent and later report the unprocessed tail finalized. DaveAppFactory derived the application address from the application parameters alone, so
newDaveAppcalled with a victim's parameters and other consensus settings could put a live application, behind an attacker-chosen DaveConsensus, at the victim's predicted address. This PR caps seals at the input span, leaves excess inputs pending, and binds the application address to the full consensus settings. It does not close the shared-factory squat: anyone can still deploy a validator-less Application at a predicted address through the shared ApplicationFactory, which makesnewDaveApprevert, so send value to a predicted address only afterDaveAppCreated. That path waits on the caller-reserved entrypoint in cartesi/rollups-contracts#561 (docs/todo.md, Before mainnet funds).On main the node never read foreclosure: it kept defending, recovering bonds and planning settlement, and never stopped. Once the dispute has a result, it now reads foreclosure and skips sentry claims, staging and acceptance while foreclosure is visible. Dispute moves and bond recovery continue, and it exits 30 only after recovery completes and foreclosure is finalized. It also pages after committing an input that makes the application terminal, and checks a candidate against the standing already read, without an extra RPC.
This is a new deployment generation. Both contract changes move deterministic addresses;
ApplicationCreatedis still emitted by the shared ApplicationFactory. The node requires a capped DaveConsensus. Earlier uncapped deployments are unsupported: the node stalls at the first seal past the span. ABI selectors are unchanged.A halt or exception is a valid transition: the node defends the true state, wins and holds it, and the guardian forecloses the application. PRT can finish normally even though DaveConsensus cannot settle that state. Foreclosure must never abandon an active dispute.
Finding dispositions
"alpha.6" refers to #287 and #299. #301 is on main but unreleased. Existing fixes are not credited to this PR.
newDaveApp. The shared-factory squat stays open pending cartesi/rollups-contracts#561 (todo, Before mainnet funds); no value beforeDaveAppCreated. New deployment generation.Retain the rejected dispositions for SLN-5, SLN-7, SLN-10, SLN-11, SLN-18, SLN-41, SLN-43 and SLN-45, and the informational dispositions for SLN-12 through SLN-14.
Cross-project changes required
No reference-node implementation or patch is included in this PR. These items remain open for the rollups-node team:
Validation and limits
just checkpassed: formatting, linting, Clippy, 447 PRT contract tests, 19 rollups contract tests, 62 Lua tests, 319 node unit tests and the rest of the Rust workspace.The input cap reverses the previous cost-based acceptance because
wasInputFinalizedmust not finalize inputs the step cannot feed. Pricing remains against the reference price: a full cap of fillers buys backlog delay for about 5.06e11 gas, roughly 3.9-5.6 times that price. The cap bounds a seal's runner work but not its drain onC, which the delay ledger counts. The spill wait draws nothing fromC, and the delay it adds is recorded as a door. Linux measurement and re-deriving the reference price remain follow-ups.Not validated: live-provider fault injection, live Arbitrum cadence, a live mempool front-running race, a full 2^24-input flood, e2e smoke, a simultaneous Go/Dave stack, or the new lifecycle harness cases on canonical two-level geometry. Contract cap tests mock input counts. The harness uses the three-level devnet. The candidate guard's pure projection and refold are tested; its context call-site has no direct regression, accepted as a guard against provider behavior outside the trust model.
Gas estimation cannot eliminate inclusion races. The node still trusts its RPC provider, including log completeness. A foreclosure racing read and inclusion can cost one transaction's gas; a node between epochs may join the next root and wait out its allowance before exiting. A fresh directory replays history, and an eliminated commitment in an abandoned root can keep waiting indefinitely.