Bug hunt on v0.123-dev: 10 fixes + ~18 reproduced bugs (failing tests) - #1704
Draft
MuncleUscles wants to merge 17 commits into
Draft
Bug hunt on v0.123-dev: 10 fixes + ~18 reproduced bugs (failing tests)#1704MuncleUscles wants to merge 17 commits into
MuncleUscles wants to merge 17 commits into
Conversation
…n infinite loop Two bugs in the terminal snapshot pruner (v0.123-dev): 1. _fetch_archive_candidates applies the already-archived anti-join AFTER the LIMIT batch_size*5 scan window. archive_once() does not prune, so archived-but-unpruned rows keep contract_snapshot NOT NULL and stay the oldest rows, permanently occupying the window. The standalone --phase archive pipeline stalls after batch_size*5 rows and falsely reports 'no eligible candidates'. New db-sqlalchemy test seeds batch_size*5+1 rows and shows only batch_size*5 ever get archived. 2. prune_terminal_snapshots --dry-run with the default --max-batches 0 never terminates: dry-run rolls back and re-fetches the same rows, so result.candidates never reaches 0 and BatchBudget never runs out. The worker loop spins, re-counting identical rows and inflating totals. New unit test drives main() and shows prune_once is called unboundedly. Both tests fail against current v0.123-dev. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
Adds failing tests reproducing three more bugs found on v0.123-dev: 1. fees: mode-1 child with budget slack can never execute. In create_child_fee_accounting (commit ab62acc) an over-funded child gets message_fee_budget = slack > 0 with empty message_allocations; when it executes, genvm_message_fee_allocation raises Mode1MessageFeesRequireGenVMPerEmissionSupport, so the child call fails before GenVM runs. Budget slack is permitted at submission, so seeding the child then rejecting it at execution is inconsistent. (tests/unit/test_studio_fees.py) 2. transactions_processor: _hydrate_archived_contract_snapshot has no error handling. A missing/unreachable/corrupt archive object makes load_snapshot raise, which propagates through get_transaction_by_hash / get_studio_transaction_by_hash and 500s every read of that transaction forever instead of degrading to contract_snapshot=None. (tests/unit/test_transactions_processor_improvements.py) 3. rpc: gen_call singleflight coalescing key is sha256(params) only, with no state-version discriminator. A read issued after a write reaches ACCEPTED can coalesce onto an older in-flight identical read and return pre-write state -- a read-your-writes violation for latest-nonfinal. (tests/unit/test_rpc_genvm_admission.py) All three tests fail against current v0.123-dev. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes 3 of the 5 bugs reproduced by the failing tests in this branch. Their regression tests now pass. 1. terminal_snapshot_pruner._fetch_archive_candidates: move the already-archived anti-join into the primary WHERE clause so it filters BEFORE the LIMIT, instead of selecting the oldest batch_size*5 rows and then excluding archived ones. Previously the standalone --phase archive pipeline stalled once the oldest batch_size*5 rows were archived (they keep contract_snapshot NOT NULL and stay the oldest rows), falsely reporting 'no eligible candidates'. The scan-window CTE and its unused candidate_scan_limit parameter are removed. 2. prune_terminal_snapshots._run_pruner_worker: stop after a dry-run batch. Dry-run rolls back instead of consuming candidates, so result.candidates never reaches 0; with the default --max-batches 0 the worker looped forever re-counting the same rows and inflating totals. Dry-run is now a bounded counting pass. 3. transactions_processor._hydrate_archived_contract_snapshot: wrap the archive load in try/except. A missing/unreachable/corrupt archive object (S3/GCS outage, absent credentials, sha256 mismatch) no longer 500s every read of that transaction; it degrades to contract_snapshot=None and logs a warning, exactly as when no archive is configured. The fees mode-1-child and gen_call singleflight stale-read repros remain as failing tests: their fixes require a maintainer design decision (see PR). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
MuncleUscles
changed the title
…sion tests Second bug-hunt wave. Four bugs with clear, low-risk fixes; each has a regression test that now passes. - vrf: get_validators_for_transaction crashed on zero-stake validators (ZeroDivisionError when total stake is 0; numpy "Fewer non-zero entries in p than size" when fewer validators have positive stake than are requested). Zero-stake validators are reachable via sim_createValidator(stake=0) and sim_config virtual validators (default stake 0), and a single one wedged every subsequent transaction's validator selection. Fall back to uniform selection in those cases. Also seed the RNG per call from os entropy: the module-level default_rng was evaluated once at import and seeded from wall-clock seconds, so processes started in the same second shared an identical, predictable stream. - providers: get_default_provider_for returned the shared cached LLMProvider on an exact match, so update_validator mutating llm_provider.config polluted the process-wide default for every later caller. Return a copy, like the fallback-template path already does. - api-keys: admin_deactivate_api_key looked up by key_prefix (only glk_ + 4 hex chars = 16 bits, not unique) and took .first(), so a prefix collision silently deactivated an arbitrary key -- an admin revoking a compromised key could disable an innocent one while the compromised key stayed active. Refuse to guess: raise on an ambiguous prefix. - accounts: debit_account_balance / credit_account_balance / credit_tx_value_once keyed current_state.id on the raw address while get_account checksum-normalizes, so crediting a lowercase address (accepted by sim_fundAccount) wrote an unreadable second row and stranded the funds. Normalize to checksum in the raw-SQL balance ops. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
…p bond Two more confirmed money bugs on v0.123-dev, left as failing repros because the fix is fee-protocol semantics the maintainers should own. 1. calculate_min_appeal_bond returns 0 for LEADER_TIMEOUT / UNDETERMINED appeals under the Studio default distribution (rotations=[0]). That branch passes the raw rotations entry as the per-round slot multiplier of _calculate_fee_for_round, but every other call site passes rotations + 1 (a 0 entry means one leader slot). Since the multiplier scales both leader and validator fees, a 0 entry zeroes the whole replacement-round cost, so the minimum bond is 0 and a third party can appeal an UNDETERMINED / LEADER_TIMEOUT tx with a zero bond -- forcing extra consensus rounds paid from the sender's pot with nothing at stake. The index target_round-1 also mismatches the round->rotations mapping used elsewhere. 2. A topUpAndSubmit appeal bond is added to primary_fee_budget AND recorded as an appeal bond, so on cancel the same money is paid out twice: once via the primary-budget refund and once via the bond payout. For submitted value 1100 + bond 1400 (collected 2500), cancel returns refund 2500 and bond payout 1400 = 3900 out for 2500 in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
… field loss
Two more v0.123-dev bugs with clear fixes and regression tests.
- can_finalize_transaction raised TypeError when
timestamp_awaiting_finalization is None. worker.py claims NULL-timestamp
rows past the stranded threshold (the defensive drain branch), but
can_finalize evaluated `time.time() - None - ...` unconditionally, so those
rows crashed process_finalization, looped through generic-error retry, and
were eventually CANCELED -- the drain branch could never succeed. Treat a
NULL timestamp as "window elapsed".
- create_snapshot omitted execution_mode, origin_address, triggered_on and
sim_config from the serialized transaction dict, while restore_snapshot
already reads them (tx_info.get("sim_config")). So sim_restoreSnapshot
silently rewrote transactions: leader-only txs came back as NORMAL (losing
the finality-window bypass), fee-settle refunds went to from_address instead
of origin_address, and per-tx sim config / triggered_on were dropped. Add
the four fields to serialization and restore the three that were missing;
also drop a duplicate "appealed" key.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
MuncleUscles
changed the title
…g tokens execute_transfer debited the sender with a read-modify-write (get_account_balance then update_account_balance, an absolute overwrite). Worker claims serialize only on to_address (pg_try_advisory_xact_lock(to_address)), so two SENDs from the same sender to different recipients (A->B and A->C) are claimed by two workers at once. Both read the sender balance before either writes, and the second commit clobbers the first's debit: the sender is debited once while both recipients are credited -- token supply inflated. Verified: two concurrent 30 + 50 sends from a balance of 100 leave the sender at 50 (30 minted) instead of 20. Use the atomic debit_account_balance (UPDATE ... WHERE balance >= amount) and credit_account_balance instead, so concurrent sends serialize on the sender row and conserve the balance. A False debit means insufficient balance (or a missing account) -> the existing UNDETERMINED short-circuit. This also removes the recipient-side create_new_account_with_address mid-flow commit() that could persist the sender debit before value_credited was set (double debit on worker-crash retry): credit_account_balance uses INSERT ... ON CONFLICT with no commit, so the debit + credit + value_credited flag land in one transaction. Adds a barrier-forced concurrency regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
MuncleUscles
changed the title
…ensus_data execute_transfer's insufficient-balance path sets status=UNDETERMINED and timestamp_awaiting_finalization but never sets consensus_data. When claim_next_finalization picks such a row up, FinalizingState.handle does `context.transaction.consensus_data.leader_receipt[0]` and raises AttributeError on NoneType, which the worker's generic-error retry loops on and eventually flips the tx to CANCELED -- downgrading a legitimately UNDETERMINED SEND to the wrong terminal status. Failing reproduction only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
…rong fields
The receipt/block formatters read dict keys that _parse_transaction_data never
produces, so eth_ JSON-RPC compatibility is broken:
- receipt.status is always 0x1 -- it does `hex(1 if tx.get("status", True)
else 0)`, but `status` is a lifecycle string ("CANCELED"/"UNDETERMINED"/...),
which is always truthy. A reverted/canceled/undetermined tx is reported to
Ethereum clients as SUCCESS.
- receipt.blockNumber and block.number/block.timestamp are always 0x0 -- they
read a non-existent `block_number`/`timestamp` key. Studio's canonical block
number is timestamp_awaiting_finalization (what get_block_by_number reports),
so confirmation counting breaks.
- receipt.contractAddress is always null even for a deployment -- it reads a
non-existent top-level `contract_address` key; the deployed address lives in
transaction["data"]["contract_address"].
Failing reproductions only (verified with a runnable repro).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
Three verified bugs in GenVM data handling, failing reproductions only.
1. calldata.encode(memoryview) emits headerless bytes (mem.extend(b.tolist()))
while every other type writes a (len<<3 | TYPE_*) header. memoryview is
explicitly special-cased and decode(memview2bytes=...) yields memoryviews
that get re-encoded, so this corrupts round-trips: encode(memoryview(b"abc"))
is 616263 not 1b616263, and a memoryview byte in a map silently decodes to a
wrong int, clobbering sibling fields.
2. _repr_result_with_capped_data is called on every GenVM execution and is
documented to fall back to a repr on failure, but its except handler re-runs
the same failing f"{result!r}". ExecutionError.__repr__ json.dumps()es
raw_error, which carries calldata-decoded bytes/Address (not JSON-
serializable), so a USER_ERROR with such data raises TypeError and aborts
_run_genvm instead of producing an error receipt.
3. EthSend PendingTransaction.calldata is populated by provide_result but
dropped by to_dict/from_dict (round-trips to b""), while _set_vote compares
full dataclass equality of pending_transactions -- so a deserialized leader
receipt vs a validator's fresh receipt mismatches on calldata alone,
yielding a spurious DETERMINISTIC_VIOLATION.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
All three are reachable through the JSON-RPC appeal path and crash-loop the
worker's generic-error retry so the tx can never re-execute. Failing repros
only.
1. PendingState's appealed branch has no new-validator fallback. After a
successful validator appeal the tx returns to PENDING with appealed=True
still set; get_validators_from_consensus_data silently drops receipts whose
addresses are no longer registered, so if the original validators are gone
involved_validators becomes [] and ProposingState raises ValueError
unpacking `[leader, *rest] = []`. The non-appealed branch already has the
correct fallback.
2. The validator-appeal timeout-replacement pool (CommittingState) is built as
"all snapshot validators minus assigned", which includes the appealed
leader (context.leader is {} in the appeal path). A timed-out juror can be
replaced by the leader whose receipt is under appeal -> the leader votes on
its own receipt, biasing the appeal. get_extra_validators already excludes
used leaders.
3. Appealing a LEADER_ONLY-accepted tx (leader_receipt length 1 sets
validation_by_leader) prepends context.leader == {} to validators_to_run;
node_factory({}) raises KeyError('address').
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
…state FinalizingState.handle sources the finalized state from the CURRENT accepted bucket (contract_snapshot_factory(...).states["accepted"]) rather than the state the finalizing transaction produced. If a younger transaction T2 was ACCEPTED after T1 (allowed while T1 is still in its finality window), T2's still-appealable writes are copied into `finalized` when T1 finalizes. finalized reads (gen_call/eth_call status="finalized") then serve state that was never finalized; if T2's appeal later succeeds, only `accepted` is rolled back, so `finalized` permanently keeps the overturned state. Failing reproduction only (verified end-to-end against Postgres with the real FinalizingState + ContractProcessor + ContractSnapshot). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
…act bricking)
Two verified state-corruption bugs reachable through the validator-appeal path.
Failing repros only (both verified end-to-end against Postgres).
1. VALIDATORS_TIMEOUT appeal re-acceptance wipes accepted state. consensus_data
is persisted with strip_contract_state=True (leader receipt contract_state
-> {}). A VALIDATORS_TIMEOUT appeal clears `appealed` and carries the
stripped receipt straight into re-acceptance (no ProposingState). Validators
AGREE via the preserved contract_state_hash, then AcceptedState writes
leader_receipt.contract_state == {} as the accepted state, overwriting the
real contract state (code slot included) with {}.
2. claim_next_appeal drops the saved pre-execution contract snapshot (the
RETURNING clause omits the contract_snapshot column), so
transaction.contract_snapshot is None on the appeal path. Downstream, appeal
validators execute from the LIVE post-tx accepted state (guaranteed hash
mismatch -> false DETERMINISTIC_VIOLATION -> correct txs' appeals spuriously
succeed), and a successful appeal restores accepted_state={} -> contract
bricked.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov
MuncleUscles
changed the title
Member
Author
|
/run-e2e studio |
Member
Author
|
/run-e2e studio |
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
A multi-wave bug hunt across the v0.123-dev backend. ~28 confirmed bugs. The first waves included fixes (10, each with a green regression test); once the ask narrowed to find-and-reproduce only, subsequent bugs are landed as failing tests that reproduce them (no fixes). Every failing test was verified to fail at the actual defect (many via a runnable repro against Postgres).
Fixed — regression test passes (10)
--phase archivestalled afterbatch_size*5rows (anti-join after theLIMIT)--dry-run(--max-batches 0) looped foreverget_default_provider_forleaked the shared cached instanceadmin_deactivate_api_keymatched a non-unique 16-bit prefix → wrong key revokedcan_finalize_transactionTypeErroron NULLtimestamp_awaiting_finalizationcreate_snapshotdroppedexecution_mode/origin_address/triggered_on/sim_configexecute_transferread-modify-write → concurrent SENDs from one sender mint tokensConfirmed & reproduced — failing tests, no fix (~18)
Consensus state / money (highest severity):
FinalizingStatecopies the live accepted bucket intofinalized, so a still-appealable younger tx's writes become permanently "finalized".tests/db-sqlalchemy/test_finalize_promotes_only_own_state.py{}) leader receipt overwrites accepted state (code slot included) with{}.tests/db-sqlalchemy/test_timeout_appeal_state_wipe.pyclaim_next_appealdrops the saved pre-execution snapshot — appeal validators execute from the live post-tx state (falseDETERMINISTIC_VIOLATION) and a successful appeal restoresaccepted_state={}→ contract bricked.tests/db-sqlalchemy/test_appeal_claim_preserves_snapshot.pyFinalizingStatecrashes onconsensus_data=None(UNDETERMINED SENDs) → retry loop → wrong CANCELED.tests/unit/consensus/test_finalizing_none_consensus_data.pyleader == {}→KeyError('address').tests/unit/consensus/test_appeal_flow_repros.pyFees / money:
topUpAndSubmitbond on cancel; over-funded mode-1 child can never execute.tests/unit/test_studio_fees.pyRPC (Ethereum compatibility):
eth_getTransactionReceipt.statusis always0x1(a failed/canceled tx reported as success);blockNumber/get_block_by_hash.number/timestampalways0x0;contractAddressalwaysnullfor deploys.tests/unit/test_transaction_receipt_formatting.pygen_callsingleflight returns stale reads (key lacks a state version).tests/unit/test_rpc_genvm_admission.pyGenVM data handling:
calldata.encode(memoryview)emits headerless bytes → silent wrong decode;_repr_result_with_capped_dataraises on non-JSONraw_error(its except re-runs the same failing repr) → aborts execution logging; EthSendPendingTransaction.calldatais dropped by serialization → spuriousDETERMINISTIC_VIOLATIONin voting.tests/unit/test_calldata_memoryview.py,test_repr_result_capped.py,tests/unit/consensus/test_eth_send_emission.pyTesting done
Python 3.12 venv + a real migrated Postgres for db-sqlalchemy tests.
No product regressions (later waves add only test files). Pre-existing-on-
v0.123-devdb failures unrelated to this branch (identical on the untouched base in this env):test_health_llm_provider_detection.py,test_finalization_starvation.py.Still flagged (not yet reproduced as tests)
TransactionLeaderRotatedemitting the pre-shufflenewLeader;temporal_snapshotswapping global LLM backends under concurrency; API-key cache invalidated before commit; worker liveness > recovery timeout; graceful-shutdown clamp; health-metrics loop-count cadence.Checks
Reviewing tips
Each test docstring states the wrong behavior, the exact trigger, and the expected behavior. The green tests are fixes + regressions; the red tests are intentional reproductions (many verified end-to-end against Postgres). The consensus state-corruption repros (
test_finalize_promotes_only_own_state,test_timeout_appeal_state_wipe,test_appeal_claim_preserves_snapshot) are the most severe — they demonstrate finalized/accepted contract state being corrupted or wiped through the appeal path.User facing release notes
Fixed: concurrent native transfers no longer mint tokens; snapshot pruning/
--dry-run; archived-snapshot read resilience; zero-stake validators no longer wedge consensus; lowercase funding addresses; ambiguous API-key revocation;sim_restoreSnapshotfield preservation; a NULL-timestamp finalization crash. The failing tests document further confirmed correctness bugs (Ethereum receipt fields, GenVM calldata, and several consensus appeal/finalization state-corruption paths) for follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01JmvcYD3JmaWXkjYn9US9ov