Conversation
471a020 to
74eeb8c
Compare
|
I'm curious, why is this PR targeting compatible? |
No good reason -- it's not correct, but this was just to try to get a quick sense of the changes involved. develop is moving fast right now because of mesa changes, so I thought the diff would stay cleaner here. I would ultimately just rebase it |
a829101 to
013662c
Compare
13b0bf8 to
ed6623d
Compare
Accounts are no longer self-delegating by default. This is the first step toward allowing accounts to opt out of staking (MIP-0010). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Public_key.Compressed.Checked.empty for circuit-level empty key checks - New accounts no longer auto-self-delegate in the fee payer or receiver paths - Delegation to the empty public key (unstaking) is allowed: - Detect unstaking via is_unstaking_tx = is_stake_delegation && receiver_is_empty - Proxy the receiver lookup through fee_payer (empty pk has no ledger entry) - Reset the merkle root after receiver update to discard the proxy side-effects - The fee payer's delegate is set to empty, which maps to None via Typ.transport - Receiver account creation no longer sets delegate to receiver's own pk Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7889d57 to
a539414
Compare
Updated for the changed transaction SNARK circuit constraints. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update zkapp_preconditions, transaction_union, and genesis_ledger tests to account for new accounts starting with delegate=None. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The non-zkApp ledger test asserted delegate was Some for default-token accounts. With delegate=None by default, assert None instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The SNARK circuit already allowed delegation to the empty public key, but the unchecked transaction logic rejected it with Receiver_not_present. Fix the unchecked path to match the SNARK: - Allow Stake_delegation to empty pk (skip receiver existence check) - Convert empty delegate to None when storing Add property-based tests (Alcotest + Quickcheck) for: - Opt-in: new account (delegate=None) delegates to a validator - Opt-out: delegating account delegates to empty, delegate becomes None - Payments between unstaked accounts preserve balances and delegate=None Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add set -eo pipefail for error detection - Sort directories deepest-first so subdirectory caches are generated before parent directories (prevents dune test caching from skipping subdirectory test runs) - Separate generation and verification into two phases with a clean _build between them (dune doesn't track env vars, so ERROR_ON_PROOF wouldn't trigger a re-run otherwise) - Only move proof_cache.json files into place after all generation succeeds Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a539414 to
55c28ee
Compare
The genesis winner must have stake to produce the first blocks. Without self-delegation its effective stake is zero and no blocks are produced.
|
!ci-nightly-me All integrations tests and dev-unit tests passing |
Add `stake_change : 'amount` to the transaction SNARK statement (Snarked_ledger_state.Poly), parallel to supply_increase. This is the first step toward maintaining a total_stake protocol parameter. - Wire types, interface, and implementation all updated - Statement merge sums stake_change (like supply_increase) - Blockchain SNARK checks stake_change equality and extracts it - All statement construction sites initialize stake_change to zero (actual computation comes in a follow-up commit) - to_input / Checked.to_input include stake_change in the hash Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Transaction_applied.stake_change which computes the per-transaction change to total_stake based on delegation status of involved accounts. Uses post-tx ledger lookups (~get_account_after) since non-delegation txs don't change delegate fields; for Stake_delegation, previous_delegate is already captured in the applied record. Wire it into staged_ledger and scan state statement construction, replacing the zero placeholders from the previous commit. Add 4 quickcheck tests (100 trials each) verifying stake_change for: - unstaked sender -> unstaked receiver payment (= 0) - staked sender -> staked receiver payment (= -fee) - opt-out delegation Some->None (= -pre_balance) - opt-in delegation None->Some (= +post_balance) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| ; ledger_hash | ||
| ; ledger_hash | ||
| ; amount | ||
| ; amount |
There was a problem hiding this comment.
I think this was accidentally duplicated
There was a problem hiding this comment.
I think it's just the type declaration for the circuit/ocaml marshling -- it's the same type as supply_increase so it looks confusing.
|
Have you seen any reason to keep tracking the total supply (sum of all mina balances) for the staged ledger and epoch ledgers in the consensus data? It seemed to me like the total supply was only needed for staking, and total delegated stake is now going to be filling that role. I know this is still a work in progress, so I understand why the total supply is there regardless. |
|
You know, I thought this was going to blow up with the unstaking change: mina/src/lib/consensus/proof_of_stake.ml Lines 50 to 55 in ed54dc3 But I see I missed the |
|
!ci-build-me |
…rator Rewrite zkapp stake_change to iterate per unique account (using zc.accounts) rather than per account_update. The per-update approach double-counted accounts appearing in multiple updates. Cap test generator max_balance to 1B MINA / num_accounts so total balances stay within Amount.t range and stake_change merges don't overflow. Update sample precomputed block serialization to include stake_change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
!ci-build-me |
|
!ci-nightly-me |
1 similar comment
|
!ci-nightly-me |
|
!ci-nightly-me |
When a coinbase has a fee_transfer, the snark worker fee is deducted from the coinbase amount: the coinbase receiver gets amount - fee and the fee_transfer recipient gets fee. The unchecked stake_change computation was using the full data.amount for the receiver, which double-counted the fee whenever both (or just the receiver) were staked. The transaction snark circuit (in transaction_snark.ml's apply_tagged internal_sc) correctly uses receiver_increase = amount - fee, so the unchecked path disagreed with the circuit and the assert_equal stake_change statement.stake_change check failed at block production time. This is why zkapps, zkapps-timing, zkapps-nonce, and payments integration tests timed out waiting for ledger proofs to be emitted: blocks containing real coinbases + ft to staked accounts could not be SNARKed. Existing coinbase unit tests did not catch this because they use Account.create which defaults delegate=None, so both the buggy unchecked path and the correct circuit computed 0 and the assertion held vacuously. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds 14 unchecked-path tests in unstaking.ml covering cases that were previously untested: - Payment: staked→unstaked, unstaked→staked, staked→new account - Stake_delegation: Some→Some redelegate (signed cmd), None→None no-op - Fee_transfer: one-single staked / unstaked, two-singles mixed - Coinbase no-ft: staked / unstaked receiver - Coinbase + ft: 4 staking combinations of (receiver, ft_receiver) Adds 3 SNARK-level tests in transaction_union.ml that exercise the circuit's assert_equal stake_change with non-zero stake values: - coinbase + ft, both receivers staked - coinbase + ft, only receiver staked - coinbase + ft, only ft_receiver staked The existing transaction_union coinbase_test only used delegate=None accounts, so both unchecked and circuit computed 0 and the assertion was vacuously satisfied. The new variants pre-create accounts with delegate=Some, forcing both sides to compute non-zero values and verify they actually agree. Together these would have caught the Coinbase + fee_transfer bug fixed in the previous commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
!ci-nightly-me |
1 similar comment
|
!ci-nightly-me |
Per the unstaking MIP, an account may have delegate = None, but Account_update.Update.delegate is a Public_key.Compressed.t Set_or_keep.t where the "unstaked" state is encoded as Public_key.Compressed.empty. get_account_update was failing with "Expected delegate in account" when reading an unstaked account. Map None to Set empty instead, matching how opt-out is encoded on the producing side (e.g. signed stake_delegation with receiver=empty, or zkapp Account.set_delegate with empty pk). Unblocks the zkapps-timing integration test, which calls get_account_update on a newly-created zkapp account whose delegate defaults to None under the new unstaking default in Account.initialize. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
!ci-nightly-me |
The per-account pre/post diff approach relied on get_account_after reading the shared mutable ledger, which in block-production order (all first passes before any second pass) is contaminated by later transactions' fee deductions on accounts the current tx also touches. This over-counted stake_change for earlier txs and caused merge-level "Statement and proof mismatch" failures against the circuit. Walk the tx's account_updates in order, tracking each touched account's (balance, delegate) state starting from zc.accounts. For staked->staked the contribution is just balance_change, so the computation no longer depends on any shared-ledger state. Failed zkapps process only the fee_payer update, since body updates are rolled back. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
!ci-nightly-me |
Adds the proofs for "Consecutive zkapps-based payments with staked accounts" introduced alongside the Transaction_applied.stake_change fix. All other proof caches in src/lib/transaction_snark/test are byte-identical and not touched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
!ci-build-me |
Replace the per-tx-type case tree (internal_sc / payment_sc /
delegation_sc / failed_sc plus four chained Amount.Signed.Checked.if_
multiplexes) with the closed-form formula documented in
docs/unstaking-stake-change.md:
total_stake_diff =
if is_user_command then signed_command_delta else internal_delta
signed_command_delta =
fee_payer_pre_balance
* (fee_payer_post_is_staked - fee_payer_pre_is_staked) (1)
- fee_amount * fee_payer_post_is_staked (2)
+ (is_payment && !user_command_fails)
* transfer_amount
* (receiver_is_staked - fee_payer_pre_is_staked) (3)
internal_delta =
fee_amount * fee_payer_pre_is_staked
+ receiver_increase * receiver_is_staked
Each "x * (a - b)" difference is encoded directly as
"gated_by a (+x) + gated_by b (-x)" instead of via explicit
opt-in/opt-out booleans, which removes five derived Boolean
bindings that previously obscured the per-term structure.
Term (1) is explicitly gated by delegate_is_being_updated so the
"only fires for stake_delegation transitions" intent is visible at
the call site, symmetric with term (3)'s explicit gating by
is_payment && !user_command_fails.
The new docs/unstaking-stake-change.md file records the derivation,
variable names, and the per-tag coverage table so the circuit and
the unchecked Transaction_applied.stake_change path can be
audited against a single source of truth.
No semantic change: 37/37 transaction_union tests pass end-to-end.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite the stake_change block in [apply_tagged_transaction] using
raw Field.Var arithmetic (one R1CS mul per [*$]) instead of
Amount.Signed.Checked combinators. The body now maps one-to-one to
the closed-form formula in docs/unstaking-stake-change.md:
stake_change =
is_user_command ? signed_command_delta : internal_delta
signed_command_delta =
delegate_is_being_updated
* fee_payer_pre_balance
* (fee_payer_post_is_staked - fee_payer_pre_is_staked) (1)
- fee_amount * fee_payer_post_is_staked (2)
+ (is_payment && not user_command_fails)
* transfer_amount
* (receiver_is_staked - fee_payer_pre_is_staked) (3)
internal_delta =
fee_amount * fee_payer_pre_is_staked
+ receiver_increase * receiver_is_staked
Changes:
- [apply_tagged_transaction] now returns [stake_change_field :
Field.Var.t] instead of an Amount.Signed.var. The outer [main]
extracts the field view of [statement.stake_change] via
[Amount.Signed.Checked.to_field_var] and asserts equality with
[Field.Checked.Assert.equal].
- All intermediate [gated_by] / [Amount.Signed.Checked.if_] /
[Amount.Signed.Checked.add] calls are replaced by direct field
operations. The derived [is_opt_in] / [is_opt_out] /
[payment_sender_active] / [payment_receiver_active] /
[signed_zero] / [as_positive_signed] / [as_negative_signed]
bindings go away entirely.
- Let-bindings are tightly scoped: [delegate_is_being_updated],
[fee_payer_post_is_staked], and [fee_payer_pre_balance] now live
inside the signed-command branch; [is_payment_body] and
[transfer_amount] live inside [payment_body_term]; and
[amt_f receiver_increase] is inlined at its one usage.
- Shared between both branches: [fee_payer_pre_is_staked],
[receiver_is_staked], [fee_amount].
Constraint count: ~10 (down from ~20). Each [*$] is one R1CS mul,
additions are free linear combinations, and the final field
equality is one constraint.
37/37 transaction_union tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements the draft proposal here in order to