PLT-1022: fund the contract deployer - #59
Open
bdchatham wants to merge 5 commits into
Open
Conversation
NewGenerator deployed every contract-backed scenario from a key it minted itself. That key held no balance and sat in no account pool, so funding never reached it and any chain that charges for gas rejected the deployment. The deploy nonce came from the scenario's index, correct only for a key that has never sent a transaction, and every failure path in DeployScenario panicked. The generator now receives its deployer. funder.Deployer resolves it: the funding root when funding is configured, otherwise a fresh key, which only a chain that credits unknown senders can pay for. Mock deploy never reads the root key, so a dry run still works with no key mounted. Each deployment leaves its nonce unset, so go-ethereum reads the deployer's pending nonce from the chain, and deployAll waits for the receipt before it sends the next one. The scenario deployments and the funder's own Disperse deployment and disperseEther batches therefore stay one ordered nonce stream on one key. Deployment failures return errors and fail the run. Tests cover the funding + contract-scenario intersection that no committed profile exercises: a deployer starting at a non-zero nonce, the single nonce stream shared with the funder, and a reverted deployment surfacing as an error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bdchatham
marked this pull request as ready for review
August 20, 2026 03:22
PR SummaryHigh Risk Overview Deployments use live chain nonces (instance index removed): sequential deploys wait for receipts, and Package docs and tests (mock JSON-RPC chain, funded multi-contract nonce ordering) document the single-nonce-stream behavior on the root key. Reviewed by Cursor Bugbot for commit 9e645a7. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…s one root Four fixes from review, each measured rather than argued. A deploy that never mined exited 0. Converting the deploy path from panic to error routed the new 30s deployTimeout into main's shutdown filter, which nulled any context.DeadlineExceeded so the run-duration timeout could exit cleanly. The filter now requires the run context itself to have expired, so a deadline raised anywhere else surfaces. endedOnRunContext carries the rule and its truth table is tested; the test fails against the sentinel-only form. The funder's WaitMined was unbounded while the deploy half was bounded, and the deploy comment claimed nothing else bounds startup. Against a chain that accepts a transaction and never mines it, funding was still blocked at 45s — and a profile need not set a run duration, so the pod stays alive and silent with no load offered. Both halves are now bounded the same way. A contract creation was priced at the load-transaction cap of 20 gwei while the funder used 100. With one shared key the deploy is the head of the root's nonce stream, so its weakest price blocked every later root transaction once the base fee rose past it. Both now use the same cap. FundAccounts takes the resolved root instead of re-reading the key file, so the deploy and funding phases cannot disagree about which identity they are — the same two-identities-where-one-is-assumed bug this change exists to fix. MockDeploy now gates funding alongside DryRun. They expressed one intent through two predicates, so a profile setting mockDeploy with funding and no --dry-run funded a pool that then transacted against code-less addresses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit made main's shutdown filter require the run context to have expired, so a deploy timeout would surface rather than exit 0. That broke the case the filter exists for: cobra runs on an uncancelled context and the signal handler reads off a channel, so a signalled run leaves ctx untouched and the context error arrives from a background task scope cancelled on the way out. Every normal SIGTERM would have reported failure. Fixed at the source instead. The filter goes back to matching the sentinels, which is what a signalled run actually produces, and DeployScenario no longer lets its own budget escape as one: an expiry of the deploy timeout is reported without a context sentinel in the chain, while a sentinel from the caller's context passes through because that really is a shutdown. The first attempt wrapped only the WaitMined call. A test against a blackhole endpoint showed the budget also escapes through the send path — every %w in the function inherits it — so the conversion moved to the function boundary. Covered by TestEndedOnRunContext, which pins a background-task cancellation with a live outer context as a clean exit, and TestDeployTimeoutIsNotAContextSentinel, which asserts the deploy error carries no sentinel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9e645a7. Configure here.
…deploy one The previous commit stopped the deploy budget escaping as a context sentinel, because main reads those as a clean exit so a signalled or duration-bounded run reports success. It left the identical alias in the funder's own wait, which the same commit had only just given a timeout: a Disperse deploy or disperseEther batch that never mined would have exited 0 as a run that funded nothing. Two sites needing one contract, so the contract is now a helper. utils.WithinBudget runs work under a deadline of its own and reports an expiry of that deadline without a sentinel in the chain, while passing a sentinel from the parent context through untouched. Both the deploy and the funding wait route through it, so a third caller inherits the rule rather than rediscovering it. Tested at the helper: a budget expiry carries no sentinel and names the budget, a parent cancellation still matches context.Canceled, and any other error keeps its chain. Verified the first fails when the stripping is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One conflict, in generator/scenarios/StorageRW_test.go, where both sides added to the same file: PLT-465 brought the contention sweep, size-bucket, op-mix, gas-floor and draw-order tests plus their helpers, and this branch added the deploy-budget sentinel test. Neither touches the other, so the resolution is the union of both plus the context import the sentinel assertion needs. main.go auto-merged cleanly and endedOnRunContext survived intact. All ten scenario tests pass, including PLT-465's gas-floor sweep against the EIP-7623 floor and this branch's assertion that a deploy budget carries no context sentinel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
The contract deployer is now a funded identity the generator receives, not one it mints. Unblocks StorageRW and every multi-contract workload on a real chain.
Defect
NewGeneratordeployed contract scenarios from a key minted insidedeployAll— before funding ran, and outsidegen.Accounts(), so funding could never reach it in any order. Failures panicked, and the deploy nonce was the instance index.No profile caught it:
arctic-1.jsonis the only funded one and runs EVMTransfer, which never deploys.Decision
funder.Deployer(cfg)names the deployer;mainhands it toNewGenerator. Funding configured → the funding root, which already deploysDisperse. Otherwise a fresh key, as today. Under mock deploy the key file is never read, so--dry-runworks with no secret mounted.Deploys leave
auth.Nonceunset and await each receipt, so scenario deploys and the funder's batches form one ordered stream on one key.DeployScenarioreturns an error instead of panicking.Tested
Mock JSON-RPC chain tracking per-sender nonces. Funding + contract scenarios — the case no profile exercises — occupies nonces 7–11 gapless. Status-0 deploy and missing-key both error. Each test fails against the defect.
Review fixes
main's run-duration filter. Now requires the run context to have expired.FundAccountsre-read the key file rather than taking the resolved root.MockDeploynow gates funding alongsideDryRun.Verifier
gofmt,go vet,golangci-lint0 issues,go test -race ./...all pass.make verifyfails only atcheck-bindings, which needs a linux-staticsolcabsent on macOS; no.solis in the diff and CI passes that gate.For review
The treasury root now signs scenario contract creations and owns them. No new capability — it already deploys
Disperse— and no load path reads that ownership, thoughDisperse.ownergates three setters. Flagging because it is the highest-value key in the system.An inherited Sei assumption now covers a different transaction. The ante handler auto-associates a sender on its first EVM transaction; that is now a scenario deploy. A sei-chain reviewer should confirm. If it does not hold, the deploy fails at admission before any spend.