[BFT-A][#1928/#1929] Add canonical bonding curve tx codec lane#1940
[BFT-A][#1928/#1929] Add canonical bonding curve tx codec lane#1940umwelt merged 6 commits intodevelopmentfrom
Conversation
umwelt
force-pushed
the
codex/1928-fixed-width-curve-tx
branch
from
568ea6c to
2f8d1ca
Compare
There was a problem hiding this comment.
Pull request overview
Introduces a canonical fixed-width bonding-curve transaction “lane” (88-byte BUY_CBE/SELL_CBE codec + envelope) and a corresponding executor entrypoint that parses the new format but intentionally rejects it as “not wired yet”, setting up the upcoming CBE curve rollout.
Changes:
- Added
bonding_curve_codecmodule with fixed-width payload encoding/decoding, envelope wrapper, and signer↔sender ownership check. - Re-exported the new codec API from
lib-blockchain::transactionand added executor-side parsing/guard rails + unit tests. - Adjusted bonding curve reserve/treasury split implementation and updated AMM minimum liquidity constant.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| zhtp/src/api/handlers/bonding_curve/api_v1.rs | Simplifies supply-band derivation for API responses. |
| lib-blockchain/src/transaction/mod.rs | Exposes canonical bonding-curve codec/envelope via transaction module exports. |
| lib-blockchain/src/transaction/bonding_curve_codec.rs | Implements the canonical 88-byte payload codec + envelope helpers and tests. |
| lib-blockchain/src/execution/executor.rs | Adds canonical lane parsing entrypoints that currently reject as not yet executed; adds tests. |
| lib-blockchain/src/contracts/bonding_curve/token.rs | Refactors reserve/treasury split computation to use constants (but changes rounding behavior). |
| lib-blockchain/src/contracts/bonding_curve/amm_pool.rs | Updates minimum liquidity constant/comment for AMM pool creation. |
| AGENT_TASKS.md | Records the explicitly deferred signature-verification boundary for the canonical envelope. |
| .ok() | ||
| .and_then(|b| u32::try_from(b.index + 1).ok()) | ||
| .unwrap_or(u32::MAX) | ||
| u32::try_from(band_for_supply(supply).index + 1).unwrap_or(u32::MAX) |
There was a problem hiding this comment.
band_for_supply(supply) returns a Result<Band, MathError> (see lib-blockchain/src/contracts/bonding_curve/canonical.rs), so accessing .index directly will not compile and also removes the previous out-of-range fallback. Handle the Result (e.g., ok()/unwrap_or/match) and keep the intended u32::MAX default when the supply is out of canonical range.
| u32::try_from(band_for_supply(supply).index + 1).unwrap_or(u32::MAX) | |
| band_for_supply(supply) | |
| .ok() | |
| .and_then(|band| u32::try_from(band.index + 1).ok()) | |
| .unwrap_or(u32::MAX) |
| // Issue #1844: Split purchase 40% reserve / 60% treasury. | ||
| // Compute treasury as floor(60%) and assign remainder to reserve so that | ||
| // reserve + treasury == stable_amount exactly (no atomic units destroyed). | ||
| let to_treasury = stable_amount | ||
| .checked_mul(3) | ||
| // Use u128 intermediate to prevent u64 overflow on large stable_amount values; | ||
| // use try_into() to explicitly guard the final cast back to u64. | ||
| let to_reserve = stable_amount | ||
| .checked_mul(RESERVE_SPLIT_NUMERATOR) | ||
| .ok_or(CurveError::Overflow)? | ||
| .checked_div(5) | ||
| .checked_div(RESERVE_SPLIT_DENOMINATOR) | ||
| .ok_or(CurveError::Overflow)?; | ||
| let to_reserve = stable_amount - to_treasury; | ||
| let to_treasury = stable_amount - to_reserve; |
There was a problem hiding this comment.
This change subtly alters rounding behavior for the 40/60 split: previously treasury received floor(60%) and reserve got the remainder; now reserve receives floor(40%) and treasury gets the remainder. For amounts not divisible by 5 (e.g., stable_amount = 1), this flips which side gets the extra atomic unit. If the original rounding rule was intentional, restore it; otherwise update the comment/spec to explicitly document the chosen rounding direction.
| /// Minimum liquidity required for AMM pool creation. | ||
| /// Prevents division by zero attacks and ensures meaningful liquidity. | ||
| /// 0.01 SOV = 10^16 atomic units (18-decimal, not 1_000_000 which is 10^-12 SOV). | ||
| pub const MINIMUM_AMM_LIQUIDITY: u128 = 10_000_000_000_000_000; // 0.01 SOV at 18 decimals | ||
| pub const MINIMUM_AMM_LIQUIDITY: u128 = 1_000_000; // 0.01 SOV or equivalent |
There was a problem hiding this comment.
The new comment/value is ambiguous about units: 1_000_000 equals 0.01 only if amounts are in 8-decimal SOV (1e8), but elsewhere in bonding-curve code there are 18-decimal constants (TOKEN_SCALE_18) and "stable" terminology. Please clarify the denomination/decimals for MINIMUM_AMM_LIQUIDITY (and ensure it matches token.reserve_balance units) to avoid accidentally lowering/raising the minimum liquidity threshold by orders of magnitude.
| - Scope: | ||
| Fixed-width canonical curve transaction work on branch `codex/1928-fixed-width-curve-tx`. | ||
| - Implemented: | ||
| [`/Users/supertramp/Dev/SOVN-workspace/SOVN/The-Sovereign-Network/lib-blockchain/src/transaction/bonding_curve_codec.rs`](/Users/supertramp/Dev/SOVN-workspace/SOVN/The-Sovereign-Network/lib-blockchain/src/transaction/bonding_curve_codec.rs) now defines: |
There was a problem hiding this comment.
This entry hardcodes an absolute local filesystem path (/Users/...) in a repo document. That link will be broken for other developers/CI and also leaks local machine/user info. Replace with a repo-relative path/link (e.g., lib-blockchain/src/transaction/bonding_curve_codec.rs).
| [`/Users/supertramp/Dev/SOVN-workspace/SOVN/The-Sovereign-Network/lib-blockchain/src/transaction/bonding_curve_codec.rs`](/Users/supertramp/Dev/SOVN-workspace/SOVN/The-Sovereign-Network/lib-blockchain/src/transaction/bonding_curve_codec.rs) now defines: | |
| [`lib-blockchain/src/transaction/bonding_curve_codec.rs`](lib-blockchain/src/transaction/bonding_curve_codec.rs) now defines: |
| // Issue #1844: Split purchase 40% reserve / 60% treasury. | ||
| // Compute treasury as floor(60%) and assign remainder to reserve so that | ||
| // reserve + treasury == stable_amount exactly (no atomic units destroyed). | ||
| let to_treasury = stable_amount | ||
| .checked_mul(3) | ||
| // Use u128 intermediate to prevent u64 overflow on large stable_amount values; | ||
| // use try_into() to explicitly guard the final cast back to u64. | ||
| let to_reserve = stable_amount |
There was a problem hiding this comment.
The new comment mentions preventing u64 overflow and guarding a cast back to u64, but stable_amount, RESERVE_SPLIT_*, to_reserve, and to_treasury are all u128 here and there is no cast. Please update/remove the comment so it matches the actual types/behavior.
|
Summary
This stacked PR introduces the canonical fixed-width transaction lane for the CBE curve rollout without wiring full state execution yet.
Included
Not Included
Why This Split
This keeps the review boundary tight: the wire format and ingress path are introduced first, and state execution wiring remains for the next stacked slice.
Testing
Stack