89: Review

2026-10-04 19:01:25 +02:00
parent f365b69e51
commit a464f80cc4
5 changed files with 35 additions and 4 deletions
@@ -15,16 +15,17 @@ See proposal.md for motivation. Raydium already discovers position NFTs and deco
 - Keep the public service a domain marker, mirroring the burner; the reference implementation additionally owns NenjimProcess startup and AutoCloseable shutdown. One scheduled executor uses fixed delay and propagates interruption across step boundaries.
 - Keep the existing shared Hub graph eager; construct/register liquidity deployment configuration and dedicated passive dependencies only at the selected liquidity entry, as reflected by the Hub/Registry deltas.
 - Build a dedicated direct Raydium instance when selected so cached pool/position state cannot trigger stale decisions. Resolve currencies through CIS and programs/precision from validated mint accounts.
 - Finish NFT discovery first, then read a fresh pool state immediately before evaluating each owned positive-liquidity position. Pass that same state to withdrawal minimum-output math. Do not reuse a price across discovery or earlier withdrawals; an unusable pool read stops the remaining withdrawals in that iteration while swap and payout remain independent.
 - Compare squared Q64.64 values by exact cross multiplication. Decimal factors cancel because pool and position share mints. For reversed ordering compare current raw price to lower-bound raw price times the retained percentage. This avoids display rounding and inversion rounding.
 - Port the official CLMM integer tick algorithm into a small Raydium math helper, reusing it for exact integer principal amounts. Existing display math remains unchanged.
- Execute decrease_liquidity_v2 (legacy and Token-2022) with owner NFT holding, personal/protocol position PDA, vaults, tick arrays, destination ATAs and bitmap extension. Create missing ATAs idempotently in the same transaction and request a 600,000 compute-unit limit for the combined operation. Account details stay in the Raydium implementation; a private legacy transaction encoder uses existing address codec, PDA and wallet primitives.
+- Execute decrease_liquidity_v2 (legacy and Token-2022) with owner NFT holding, personal/protocol position PDA, vaults, tick arrays, owner destination token accounts and bitmap extension. Reuse an existing owner holding for each pool/reward mint, validating its account's mint, owner, program and initialized state. Only fall back to an existing or idempotently created ATA when no holding exists; failed holdings discovery must not create an extra account. This preserves the existing wallet's single-account-per-mint contract. Request a 600,000 compute-unit limit for the combined operation. Account details stay in the Raydium implementation; a private legacy transaction encoder uses existing address codec, PDA and wallet primitives.
 - Validate pool and personal-position discriminators, token metadata and owner holding before signing. Apply explicit slippage to integer expected principal. Fail closed on unsupported transfer-affecting Token-2022 extensions rather than silently weakening minimums. Required reward accounts, if present, only collect into this wallet and never enter swaps/payouts.
 - Parse only conf/evelyn-liquidity.conf with format-version validation, unknown/duplicate key rejection and required keys. Deployment addresses/signer remain placeholders in the ignored local runtime file and tracked example. Existing Hub example stays unchanged.
 - Use controlled JUnit fixtures explicitly requested by issue #89, including transaction bytes. No test constructs a real signer or submits a financial operation.
 ## Risks / Trade-offs
- [Pool moves between read and execution] → Minimum principal limits cause atomic failure; later iterations reread state.
+- [Pool moves during discovery or earlier withdrawals] → Refresh price and eligibility for each position. Minimum principal limits protect received principal, not the harvest-distance threshold. Market movement after the final decision remains possible before execution; later iterations reread state.
 - [Unknown transaction outcome] → Log signature when available, preserve ordinary underlying confirmation; subsequent iterations use current balances/positions as requested.
 - [Mint extension or program layout changes] → Validate discriminators/layout and fail safely on unsupported extensions; document official source references and exact supported behavior.
 - [SOL starvation] → Document funding for transaction fees and token-account rent; no hidden token reserves.
@@ -11,6 +11,7 @@ Evelyn needs to realize proceeds from manually funded EVE/SyrupUSDC CLMM positio
 - Add strict versioned deployment configuration and a public placeholder example with both reserves zero.
 - Lazily construct dedicated liquidity wallet, Jupiter and direct Raydium dependencies only for the selected process.
 - Verify with controlled offline fixtures and document operation/recovery.
 - Correct the review findings from commit `7dfd43d`: refresh the pool price for each position after discovery and reuse existing owner token accounts before creating destination ATAs.
 ## Capabilities
@@ -5,7 +5,7 @@ Realize Evelyn's manually funded EVE/SyrupUSDC concentrated liquidity into USDC
 ## ADDED Requirements
 ### Requirement: Owned positions qualify by their normalized upper boundary
-The service SHALL discover positions owned by its wallet in its configured CLMM pool and withdraw positive liquidity only when the upper EVE/SyrupUSDC boundary U satisfies U <= P*(1-h/100). P SHALL come from current on-chain pool state, both orientations SHALL be normalized, and decisions SHALL use pool precision. Unusable P SHALL skip withdrawals while allowing later steps.
+The service SHALL discover positions owned by its wallet in its configured CLMM pool and withdraw positive liquidity only when the upper EVE/SyrupUSDC boundary U satisfies U <= P*(1-h/100). P SHALL come from a fresh on-chain pool read for each owned positive-liquidity position after NFT discovery and previous withdrawals have completed. The same state SHALL be used for that position's eligibility decision and withdrawal quote; an earlier position's price SHALL NOT be reused. Both orientations SHALL be normalized, and decisions SHALL use pool precision. Unusable P SHALL skip the remaining withdrawals in that iteration while allowing later steps.
 #### Scenario: Inclusive threshold and upper boundary
 - **WHEN** P is 15, h is 10 and positions span 13–13.5 and 13.5–14
@@ -19,6 +19,14 @@ The service SHALL discover positions owned by its wallet in its configured CLMM
 - **WHEN** a position belongs to another wallet or pool, has zero liquidity, or its upper boundary exceeds the threshold after a price decline
 - **THEN** it retains its state and the service neither withdraws it nor creates or refills positions
 #### Scenario: Price declines during discovery or an earlier withdrawal
 - **WHEN** a position qualifies before discovery or an earlier withdrawal, but its upper boundary exceeds the threshold at the new pool price
 - **THEN** the service reads that new price before deciding and preserves the position's liquidity in either token orientation
 #### Scenario: Pool read fails between positions
 - **WHEN** an earlier position was withdrawn but the next current pool price is unavailable or unusable
 - **THEN** no remaining position is withdrawn in that iteration and swap and payout still run on their independent balances
 ### Requirement: Swap and payout use independent current balances
 The service SHALL read SyrupUSDC at step 2 and USDC at step 3 and operate on the entire respective balance above its nonnegative reserve. Absent or at/below-reserve balances SHALL be skipped. Exact-input swap SHALL use explicit maximum slippage. SPL payout SHALL target the configured wallet owner with existing account-creation support. Amounts SHALL never round upward. EVE and reward tokens SHALL remain untouched by this pipeline.
@@ -5,7 +5,7 @@ Provide a narrowly scoped protected Raydium CLMM full liquidity decrease using t
 ## ADDED Requirements
 ### Requirement: Full withdrawal preserves the reusable position
-The operation SHALL validate the NFT owner, pool and positive current liquidity, decrease that complete liquidity into the same wallet, collect pool-token fees and preserve the position account, NFT and NFT token account. Transaction/account details SHALL remain behind Raydium. It SHALL submit once and use existing confirmation behavior without close, burn or service-level retry.
+The operation SHALL validate the NFT owner, pool and positive current liquidity, decrease that complete liquidity into the same wallet, collect pool-token fees and preserve the position account, NFT and NFT token account. For each pool or reward mint it SHALL reuse an existing owner token holding, including a non-associated account, after validating its mint, owner, program and initialized state. It SHALL use or create an ATA only when no owner holding exists and SHALL NOT fall back to account creation when holdings discovery fails. Transaction/account details SHALL remain behind Raydium. It SHALL submit once and use existing confirmation behavior without close, burn or service-level retry.
 #### Scenario: Full protected withdrawal
 - **WHEN** a wallet withdraws its position's complete liquidity
@@ -15,6 +15,14 @@ The operation SHALL validate the NFT owner, pool and positive current liquidity,
 - **WHEN** ownership, pool or current position state is inconsistent with the requested withdrawal
 - **THEN** the operation fails before signing
 #### Scenario: Existing non-associated token accounts
 - **WHEN** the wallet already has a pool-token or reward holding outside its ATA under either supported token program
 - **THEN** the withdrawal targets that validated account and creates no additional account for that mint, preserving subsequent wallet balance reads
 #### Scenario: Existing holding cannot be trusted
 - **WHEN** holdings discovery fails or the selected account has a wrong mint, owner, token program or initialized state
 - **THEN** withdrawal fails before signing without creating a replacement token account
 ### Requirement: Minimum output derives from precise state and explicit slippage
 Withdrawal minimums SHALL derive from current Q64.64 pool state, exact CLMM tick/liquidity math and configured slippage with downward raw-token rounding. Both minimums SHALL NOT be unconditionally zero. Required legacy/Token-2022 accounts, tick arrays and bitmap extension SHALL follow the official decrease instruction. Unsupported token extensions SHALL fail safely before signing.
@@ -16,6 +16,12 @@
 - [x] 3.1 Lazily register the optional process and dedicated wallet/Jupiter/direct Raydium dependencies; verify optional selection preserves existing startup plan and checked-in example inactivity.
 - [x] 3.2 Compile main/verification sources, run affected offline tests, validate OpenSpec strictly and run git diff --check; record results without live financial operations, commits or pushes.
 ## 4. Review corrections for commit 7dfd43d
 - [x] 4.1 Refresh pool state per position after discovery and previous withdrawals; verify price declines in both orientations, quote consistency and independent later steps after a pool-read failure.
 - [x] 4.2 Reuse validated existing pool/reward token holdings before creating ATAs; verify both token programs, invalid accounts, failed holdings reads and absence of duplicate-account creation.
 - [x] 4.3 Update operation documentation, compile and run all offline tests, validate OpenSpec strictly and check the diff without live financial operations, commits or pushes.
 ## Verification result (2026-10-04)
 - Main and verification compilation passed with JavaFX JDK 25 and Gradle 9.6.1.
@@ -23,3 +29,10 @@
 - Strict OpenSpec change validation passed; git diff --check and new-file whitespace checks passed.
 - Public mainnet inspection was read-only. All financial operation verification used controlled offline dependencies; no live withdrawal, swap, transfer, signer invocation, commit or push.
 - Runtime configuration contains deployment placeholders and is ignored by Git. Both example reserves are zero; the Hub runtime selection and checked-in Hub example are unchanged.
 ## Review correction verification (2026-10-04)
 - Eight new regression tests reproduced the findings before their respective fixes and passed afterward: four cover per-position price freshness, quote consistency and independent later steps; four cover existing pool/reward accounts, invalid accounts and failed holdings reads.
 - Main/verification compilation and all 42 repository JUnit tests passed with no failures, errors or skips.
 - Strict OpenSpec validation and git diff --check passed. The active change has 10/10 completed tasks.
 - Verification used controlled offline dependencies only. No live financial operation, real signer invocation, commit, push or deployment was performed.