-
Notifications
You must be signed in to change notification settings - Fork 41
Add mina-tx-type crate with coinbase transaction types #1833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
OCaml Reference Validation ResultsRepository: https://github.com/MinaProtocol/mina.git Click to see full validation output |
This adds a new standalone crate that provides Mina Protocol transaction types for external use without requiring the full ledger crate. Extracted types and traits: - Sgn enum (sign type) - Magnitude trait (numeric operations) - MinMax trait (bounds) - Signed<T> struct (signed magnitude wrapper) - Amount and Fee types (via impl_number! macro) - Coinbase and CoinbaseFeeTransfer structs The extraction excludes proof-related code (to_field, of_field, etc.) to keep the crate lightweight. The ledger crate continues to use its own implementations.
60e4851 to
5937ba6
Compare
✓ Code Reference Verification PassedAll code references in the documentation have been verified successfully! Total references checked: 1 The documentation is in sync with the codebase on the |
This commit adds the proof-related types needed for the Magnitude trait's field conversion methods: - proofs/field.rs: FieldWitness trait, Boolean, CircuitVar, ShiftedValue, ShiftingValue, and related types - proofs/to_field_elements.rs: ToFieldElements trait - proofs/witness.rs: Check trait and Witness struct The Magnitude trait now includes to_field() and of_field() methods that match the ledger crate's implementation. Dependencies added: ark-ec, ark-ff, kimchi, mina-curves, mina-p2p-messages, mina-poseidon, poseidon
Next Steps: Trait Replacement OrderThe next step is to replace the traits in the ledger crate with imports from
Why start with
|
ToFieldElements Migration StatusI attempted to replace the What was accomplishedIn mina-tx-type:
In ledger:
Why full replacement wasn't possibleRust's orphan rules prevent implementing a foreign trait for combinations involving foreign types. Since both Steps needed to complete the migration
See the documentation comments in |
- Add comprehensive ToFieldElements implementations to mina-tx-type for generic types (Vec, Box, arrays, tuples, primitives) and external types (mina_signer, mina_p2p_messages) - Add field_to_bits and field_of_bits helper functions - Add migration documentation to ledger/src/proofs explaining orphan rule limitations and steps needed to complete trait replacement - Document future migration to snarky-rs and pickles-rs crates (issues #1762, #1763)
Extend the impl_number macro in mina-tx-type to generate ToFieldElements and Check trait implementations for currency types (Amount, Fee). This matches the implementations in the ledger crate. Note: The Check implementation is simplified as it doesn't include the full range checking (to_field_checked_prime) which depends on the transaction module. This will be properly implemented when snarky-rs is available.
Migration Plan: Using mina-tx-type Types in LedgerCoinbase Types Migration AnalysisI analyzed whether the Current Status: Blocked The ledger versions have additional methods that depend on ledger-specific types:
|
Update: Coinbase Type Migration Blocked by Currency TypesAfter attempting to use The ProblemThe // mina-tx-type/src/coinbase.rs
use crate::currency::{Amount, Fee}; // mina_tx_type::currency typesWhile the ledger crate uses: // ledger/src/scan_state/currency.rs
pub struct Amount(pub u64); // Different type
pub struct Fee(pub u64); // Different typeThese are incompatible types. Even though they have the same structure, Rust treats them as completely different types. Why This MattersAll existing code in ledger that works with
Simply importing Required Migration PathTo properly migrate Coinbase types, we must first:
Complexity AssessmentThe currency types are used extensively throughout the ledger crate. This migration would affect:
RecommendationThis PR establishes The current PR can be merged as-is - it provides a clean foundation for future migration. |
Decision: Keep Currency Types Separate for NowAfter further analysis, migrating
This is a significant refactoring effort that would affect many files across the ledger crate. Current StateThis PR establishes
Future Migration PathThe types in
RecommendationMerge this PR to establish the foundation. The actual migration should be done in dedicated follow-up PRs with proper testing and review due to the scope of changes required. |
Add mina-tx-type workspace dependency to the ledger crate to prepare for future migration of shared types. The actual migration of Coinbase and currency types will be done in follow-up PRs due to the extensive changes required.
Migrate Amount and Fee types from ledger's scan_state/currency.rs to use the types defined in mina-tx-type crate. This involves: - Re-exporting Amount, Fee, Magnitude, MagnitudeFieldExt, MinMax, Sgn, and Signed from mina-tx-type in ledger's currency module - Creating extension traits to work around Rust's orphan rules: - AmountFeeFieldExt<F> for field conversion methods - ToCheckedExt<F> and SignedToCheckedExt<F,T> for checked conversions - SignedExt<T> for random generation - SgnExt for sign field conversion - Splitting impl_nat! macro into impl_nat_32! and impl_nat_64! variants to handle different bit widths properly - Adding inner_to_field and inner_of_field methods to CheckedCurrency and CheckedNat traits for field conversions - Updating ForZkappCheck trait to use zkapp_to_field method - Adding necessary imports across affected files This is part of the effort to extract common transaction types into the mina-tx-type crate for reuse across the codebase.
Summary
mina-tx-typecrate with standalone transaction types for external useCoinbase,CoinbaseFeeTransfer) with supporting currency typesMagnitudetrait,Signed<T>wrapper, andimpl_number!macroDetails
This PR creates a new crate that provides Mina Protocol transaction types without requiring the full ledger crate dependencies. This allows external projects to work with Mina transactions in a lightweight manner.
Extracted components
Traits:
Magnitude- Core numeric operations (simplified, without proof-related methods)MinMax- Min/max value boundsTypes:
Sgn- Sign enum (Pos/Neg)Signed<T>- Signed magnitude wrapperAmount- Currency amount typeFee- Transaction fee typeCoinbase- Coinbase transactionCoinbaseFeeTransfer- Fee transfer to SNARK workerMacro:
impl_number!- Generates currency types with trait implementationsWhat was NOT extracted (intentionally)
Proof-related code stays in the ledger crate:
to_field()/of_field()methodsToFieldElements,Check,ToInputstrait implementationsto_bits()methodDocumentation
Added
mina-tx-type/EXTRACTION_ANALYSIS.mdwith detailed analysis of:Test plan
cargo test -p mina-tx-type)cargo check -p mina-tree)make format)Closes #1824