feat(platform-wallet): address funding fee quote FFI and Swift wrapper - #4447
feat(platform-wallet): address funding fee quote FFI and Swift wrapper#4447llbartekll wants to merge 1 commit into
Conversation
…apper Expose the state-aware getAddressFundingFeeQuote through the wallet FFI and the Swift SDK, following the existing handle-bound async patterns: - platform_address_wallet_quote_funding_fee(handle, recipient_address, recipient_address_len, prepared_outpoint, user_fee_increase, out_quote): resolves the wallet from PLATFORM_ADDRESS_WALLET_STORAGE and runs PlatformAddressWallet::quote_funding_fee on the worker runtime (the wallet already owns its SDK — no separate SDK handle). prepared_outpoint is a nullable *const OutPointFFI — NULL lets the node use its deterministic placeholder; a non-null outpoint is serialized with the chain's exact key layout (raw txid bytes plus the vout in little endian) via dashcore's OutPoint conversion. The out parameter is plain data (nothing to free), zeroed as a sentinel before any fallible step. Guard tests pin the fail-closed codes: null pointers, malformed address bytes, unknown handle. - Swift: ManagedPlatformAddressWallet.quoteFundingFee(recipientAddress: preparedOutpointTxid:preparedOutpointVout:userFeeIncrease:) async throws -> AddressFundingFeeQuote, via the established Task.detached wrapper. The docs state the quote is planning data from a single node (no proof) and that lock sizing stays governed by minimumRequiredLockCredits plus the app's own margin policy. There is no offline fallback anywhere in the path — a network failure throws, never a stale constant. Verified end to end with build_ios.sh --target sim --profile dev: the regenerated platform-wallet-ffi header carries the new symbol and SwiftExampleApp (compiling the full SwiftDashSDK) builds. dashwallet-ios is intentionally untouched at this stage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml 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:
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 |
|
✅ Final review complete — no blockers (commit 6f4fda0) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The FFI and Swift wrapper follow the established handle-storage, worker-runtime, and result-ownership patterns, with no blocking correctness or memory-safety issues found. Two non-blocking gaps remain: the public Swift API does not specify the required txid byte order, and the prepared-outpoint serialization branch lacks regression coverage.
Source: Codex general and FFI-specialist reviewers (exact backend model identifier was not included in the supplied evidence); grok-4.5 final verifier backend. Orchestration only: openclaw-agent/cliproxy/gpt-5.6-sol, not reviewer evidence.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
🟡 2 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformAddressWallet.swift:938-941: Specify the required txid byte order in the public Swift API
The wrapper copies `preparedOutpointTxid` byte-for-byte into `OutPointFFI`, and Rust passes those bytes to `Txid::from_byte_array` before serializing the Platform outpoint key. The parameter therefore requires the raw little-endian wire representation rather than the customary display-order txid. The sibling `resumeFundFromAssetLock` API documents this requirement explicitly. Without equivalent documentation here, a caller can reverse the wrong bytes and query a different outpoint, causing a spent prepared lock to appear absent instead of being rejected.
In `packages/rs-platform-wallet-ffi/src/platform_addresses/quote.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/platform_addresses/quote.rs:82-93: Add a regression test for prepared-outpoint serialization
The guard test supplies a null `prepared_outpoint` in every call, leaving the new non-null conversion branch untested. This branch defines an endian-sensitive chain key used to detect an already-consumed asset lock. Add an offline unit test with a patterned 32-byte txid and a non-symmetric vout, asserting that conversion produces exactly `raw_txid || vout.to_le_bytes()`. This will catch accidental txid reversal or native-endian vout serialization without requiring a network-backed wallet.
| /// - preparedOutpointTxid: the 32-byte txid of an already built and | ||
| /// signed lock transaction, when the wallet has one; `nil` lets the | ||
| /// node use a deterministic placeholder with the same expected | ||
| /// search depth. An outpoint already spent on Platform is rejected. |
There was a problem hiding this comment.
🟡 Suggestion: Specify the required txid byte order in the public Swift API
The wrapper copies preparedOutpointTxid byte-for-byte into OutPointFFI, and Rust passes those bytes to Txid::from_byte_array before serializing the Platform outpoint key. The parameter therefore requires the raw little-endian wire representation rather than the customary display-order txid. The sibling resumeFundFromAssetLock API documents this requirement explicitly. Without equivalent documentation here, a caller can reverse the wrong bytes and query a different outpoint, causing a spent prepared lock to appear absent instead of being rejected.
| /// - preparedOutpointTxid: the 32-byte txid of an already built and | |
| /// signed lock transaction, when the wallet has one; `nil` lets the | |
| /// node use a deterministic placeholder with the same expected | |
| /// search depth. An outpoint already spent on Platform is rejected. | |
| /// - preparedOutpointTxid: the 32-byte raw txid in little-endian wire | |
| /// order (the same byte order as `OutPointFFI.txid`) of an already | |
| /// built and signed lock transaction. If sourced from display-order | |
| /// hex, decode it back to raw order before passing it here. `nil` lets | |
| /// the node use a deterministic placeholder with the same expected | |
| /// search depth. An outpoint already spent on Platform is rejected. |
source: ['codex']
| let outpoint: Option<[u8; 36]> = if prepared_outpoint.is_null() { | ||
| None | ||
| } else { | ||
| let outpoint_ffi = *prepared_outpoint; | ||
| // The same byte layout the chain uses for outpoint keys: | ||
| // raw txid bytes followed by the vout in little endian. | ||
| let out_point = dashcore::OutPoint { | ||
| txid: dashcore::Txid::from_byte_array(outpoint_ffi.txid), | ||
| vout: outpoint_ffi.vout, | ||
| }; | ||
| Some(out_point.into()) | ||
| }; |
There was a problem hiding this comment.
🟡 Suggestion: Add a regression test for prepared-outpoint serialization
The guard test supplies a null prepared_outpoint in every call, leaving the new non-null conversion branch untested. This branch defines an endian-sensitive chain key used to detect an already-consumed asset lock. Add an offline unit test with a patterned 32-byte txid and a non-symmetric vout, asserting that conversion produces exactly raw_txid || vout.to_le_bytes(). This will catch accidental txid reversal or native-endian vout serialization without requiring a network-backed wallet.
source: ['codex']
2 participants
Issue being fixed or feature implemented
Final PR of the address-funding fee-quote stack (4/4, on top of #4446). Exposes the quote to iOS through the wallet FFI and the Swift SDK, so dashwallet-ios can later replace its static funding-fee heuristic with a state-aware network quote. dashwallet-ios itself is intentionally untouched at this stage.
What was done?
platform_address_wallet_quote_funding_fee(handle, recipient_address, recipient_address_len, prepared_outpoint, user_fee_increase, out_quote)— resolves the wallet fromPLATFORM_ADDRESS_WALLET_STORAGEand runsPlatformAddressWallet::quote_funding_feeon the worker runtime (the wallet already owns its SDK — no separate SDK handle).prepared_outpointis a nullable*const OutPointFFI— NULL lets the node use its deterministic placeholder; a non-null outpoint is serialized with the chain's exact key layout (raw txid bytes plus vout in little endian) via dashcore'sOutPointconversion. The out parameter is plain data (nothing to free), zeroed as a sentinel before any fallible step.ManagedPlatformAddressWallet.quoteFundingFee(recipientAddress:preparedOutpointTxid:preparedOutpointVout:userFeeIncrease:) async throws -> AddressFundingFeeQuote, via the establishedTask.detachedwrapper. The docs state the quote is planning data from a single node (no proof) and that lock sizing stays governed byminimumRequiredLockCreditsplus the app's own margin policy. There is no offline fallback anywhere in the path.How Has This Been Tested?
ErrorInvalidParameter), unknown handle (NotFound).build_ios.sh --target sim --profile dev: the regenerated platform-wallet-ffi header carries the new symbol and struct, and SwiftExampleApp (compiling the full SwiftDashSDK, including the new wrapper) builds successfully.Breaking Changes
None — additive.
Checklist:
🤖 Generated with Claude Code