feat(platform-wallet): pure transaction decoder + thin FFI/Swift wrappers#3981
feat(platform-wallet): pure transaction decoder + thin FFI/Swift wrappers#3981llbartekll wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Rust ChangesTransaction Decode Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftCaller
participant TransactionDecoder
participant platform_wallet_decode_transaction
participant platform_wallet_decoded_transaction_free
SwiftCaller->>TransactionDecoder: decode(txData, network)
TransactionDecoder->>platform_wallet_decode_transaction: tx bytes, network
platform_wallet_decode_transaction->>platform_wallet_decode_transaction: deserialize transaction, derive addresses
platform_wallet_decode_transaction-->>TransactionDecoder: DecodedTransactionFFI pointer
TransactionDecoder->>TransactionDecoder: convert pointers to Data/String/arrays
TransactionDecoder->>platform_wallet_decoded_transaction_free: free decoded pointer (defer)
TransactionDecoder-->>SwiftCaller: DecodedTransaction
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
llbartekll
force-pushed
the
feat/core-tx-decode-ffi
branch
from
574363a to
5620fb0
Compare
llbartekll
changed the title
…wrappers Host apps that consume the platform-wallet FFI need to match arbitrary persisted transactions against external addresses and exact amounts (e.g. the CrowdNode on-chain API: "did a tx pay amount X to address Y?"). The wallet layer persists every transaction's raw bytes but only materializes TXO rows for the wallet's own outputs, so matching an external payment's per-output (address, amount) was unevaluable from either the FFI or the Swift SDK. Layered so the logic lives in the pure crate and the FFI stays a marshalling shell (mirroring how mnemonic_words.rs forwards to key_wallet::Mnemonic): - rs-platform-wallet/src/transaction_decode.rs (pure, no FFI, no unsafe): decode_transaction(bytes, network) -> DecodedTransaction — consensus-decode (rejecting trailing bytes), per-output (Address::from_script, value, script), per-input (OutPoint + best-effort P2PKH sender address from the scriptSig). 6 pure-Rust unit tests. - rs-platform-wallet-ffi/src/tx_decode.rs: platform_wallet_decode_transaction / ..._free — thin C-ABI marshalling over the pure fn + #[repr(C)] structs. 3 boundary tests (C round-trip, null/empty params, null-free safety). - swift-sdk TransactionDecoder.swift: typed DecodedTransaction wrapper with PlatformWalletError mapping, defer-freed FFI memory, txidDisplayHex helper. 7 unit tests. Purely additive — no breaking changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
llbartekll
force-pushed
the
feat/core-tx-decode-ffi
branch
from
5620fb0 to
7cb62fe
Compare
llbartekll
requested review from
QuantumExplorer,
ZocoLini,
lklimek and
shumkov
as code owners
|
✅ Review complete (commit ffa8c94) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/swift-sdk/SwiftTests/SwiftDashSDKTests/TransactionDecoderTests.swift (2)
58-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove tautological assertion.
Line 61's
XCTAssertEqual(decoded.txid, Data(decoded.txid.reversed().reversed()))is always true by construction (double-reversal restores the original) — it doesn't verify anything aboutdecoded.txid's correctness. The line above it (comparingtxidDisplayHex) already does the real work.🧹 Proposed fix
func testTxidDisplayHexMatchesExplorerOrder() throws { let decoded = try TransactionDecoder.decode(fixtureData, network: .testnet) XCTAssertEqual(decoded.txidDisplayHex, Self.fixtureTxidDisplay) - XCTAssertEqual(decoded.txid, Data(decoded.txid.reversed().reversed())) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/TransactionDecoderTests.swift` around lines 58 - 62, The test in TransactionDecoderTests.testTxidDisplayHexMatchesExplorerOrder contains a tautological assertion that always passes, so remove the redundant XCTAsertEqual on decoded.txid and keep the meaningful check against Self.fixtureTxidDisplay; if you want additional coverage, replace it with an assertion that compares decoded.txid to an independently derived expected value from the fixture rather than reversing the same data twice.
70-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the specific thrown error, not just "any error".
testMalformedBytesThrowDeserializationandtestEmptyInputThrowsInvalidParameteronly check that some error is thrown, but their names (andTransactionDecoder.decode's documented contract of.deserializationfor malformed bytes /.invalidParameterfor empty input) imply verifying the specific case. As written, a regression that throws the wrong error type would pass silently.🧪 Proposed fix
func testMalformedBytesThrowDeserialization() { var bytes = fixtureData bytes.append(contentsOf: [0xDE, 0xAD]) // trailing garbage - XCTAssertThrowsError(try TransactionDecoder.decode(bytes, network: .testnet)) - XCTAssertThrowsError(try TransactionDecoder.decode(Data([0xFF, 0xFF, 0xFF]), network: .testnet)) + XCTAssertThrowsError(try TransactionDecoder.decode(bytes, network: .testnet)) { error in + guard case PlatformWalletError.deserialization = error else { + return XCTFail("Expected .deserialization, got \(error)") + } + } + XCTAssertThrowsError(try TransactionDecoder.decode(Data([0xFF, 0xFF, 0xFF]), network: .testnet)) { error in + guard case PlatformWalletError.deserialization = error else { + return XCTFail("Expected .deserialization, got \(error)") + } + } } func testEmptyInputThrowsInvalidParameter() { - XCTAssertThrowsError(try TransactionDecoder.decode(Data(), network: .testnet)) + XCTAssertThrowsError(try TransactionDecoder.decode(Data(), network: .testnet)) { error in + guard case PlatformWalletError.invalidParameter = error else { + return XCTFail("Expected .invalidParameter, got \(error)") + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/TransactionDecoderTests.swift` around lines 70 - 79, The tests in TransactionDecoderTests are only asserting that TransactionDecoder.decode throws, so they can miss regressions where the wrong error is returned. Update testMalformedBytesThrowDeserialization and testEmptyInputThrowsInvalidParameter to inspect the thrown error and verify the specific TransactionDecoder error case matches the contract: malformed/trailing bytes should map to deserialization and empty input should map to invalidParameter. Use the existing TransactionDecoder.decode call sites in these tests to locate the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/swift-sdk/SwiftTests/SwiftDashSDKTests/TransactionDecoderTests.swift`:
- Around line 58-62: The test in
TransactionDecoderTests.testTxidDisplayHexMatchesExplorerOrder contains a
tautological assertion that always passes, so remove the redundant XCTAsertEqual
on decoded.txid and keep the meaningful check against Self.fixtureTxidDisplay;
if you want additional coverage, replace it with an assertion that compares
decoded.txid to an independently derived expected value from the fixture rather
than reversing the same data twice.
- Around line 70-79: The tests in TransactionDecoderTests are only asserting
that TransactionDecoder.decode throws, so they can miss regressions where the
wrong error is returned. Update testMalformedBytesThrowDeserialization and
testEmptyInputThrowsInvalidParameter to inspect the thrown error and verify the
specific TransactionDecoder error case matches the contract: malformed/trailing
bytes should map to deserialization and empty input should map to
invalidParameter. Use the existing TransactionDecoder.decode call sites in these
tests to locate the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16940c17-0bcc-4af7-921e-ed66b8b231c9
📒 Files selected for processing (6)
packages/rs-platform-wallet-ffi/src/lib.rspackages/rs-platform-wallet-ffi/src/tx_decode.rspackages/rs-platform-wallet/src/lib.rspackages/rs-platform-wallet/src/transaction_decode.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/TransactionDecoder.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/TransactionDecoderTests.swift
There was a problem hiding this comment.
Code Review
Small, additive PR adding a pure transaction decoder in rs-platform-wallet plus a thin FFI/Swift wrapper. FFI memory management is correctly paired (Box::into_raw / Vec::from_raw_parts / CString round-trips), no consensus surfaces are touched, and tests cover the happy path plus trailing-garbage/coinbase/malformed cases. No blocking issues. Main suggestions cluster around the P2PKH sender-address heuristic (which infers a sender from the last scriptSig push without verifying P2PKH shape, so P2SH spends could occasionally be misattributed) and a couple of small performance/defensive tightenings.
🟡 5 suggestion(s) | 💬 2 nitpick(s)
Source: reviewers: general(opus, claude-sonnet-5, gpt-5.5[high] failed/unparseable), security-auditor(opus, claude-sonnet-5, gpt-5.5[high] failed/unparseable), rust-quality(opus, claude-sonnet-5, gpt-5.5[high] failed/unparseable), ffi-engineer(opus, claude-sonnet-5, gpt-5.5[high] failed/unparseable); verifier: opus.
🤖 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/rs-platform-wallet/src/transaction_decode.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/transaction_decode.rs:96-111: P2PKH sender heuristic doesn't verify scriptSig shape, only the last push's length
`input_address_from_script_sig` iterates every instruction in the scriptSig and takes whichever data push occurred last, then treats it as a pubkey if it is 33 or 65 bytes. It never checks that the scriptSig actually has the P2PKH shape (exactly two pushes: signature then pubkey). For a P2SH spend the last push is the redeem script; if that redeem script happens to be 33 or 65 bytes and parses as a valid secp256k1 point (spender-controllable, only constrained by hashing to the P2SH address), the function silently returns a P2PKH address unrelated to the actual signer. The module doc claims `None` for non-P2PKH, but the implementation doesn't uphold that.
The simplest tightening is to require exactly two pushes (`sig`, `pubkey`) — anything else, including multisig P2SH redeem scripts, returns `None`. An additional guard that the first push looks like a DER-encoded ECDSA signature (starts with `0x30`, length ≤ 73) would eliminate essentially all remaining collision risk. This is not blocking for the stated CrowdNode use case (which matches on outputs, not input senders), but the failure mode — a wrong non-null address is worse than `None` — is worth closing since callers building sender-verification logic on top of `input.address` would be fooled.
- [SUGGESTION] packages/rs-platform-wallet/src/transaction_decode.rs:76-90: Unnecessary clone of every output's scriptPubkey
`decode_transaction` iterates `tx.output` by reference and clones every `script_pubkey` to populate `DecodedOutput::script_pubkey`. Since `tx.txid()` only needs `&self` and doesn't care about the order relative to consuming other fields, the txid can be computed first, after which `tx.output` (and `tx.input`) can be consumed via `into_iter()` — eliminating the per-output `ScriptBuf` clone entirely. For transactions with large scripts (OP_RETURN payloads, multisig redeem scripts) this is a real, avoidable allocation on every decode call, and this function is positioned as the primitive for scanning arbitrary persisted transactions.
- [SUGGESTION] packages/rs-platform-wallet/src/transaction_decode.rs:113-228: No test exercises a non-P2PKH scriptSig (the documented `None` path)
The only "no address" input test is `coinbase_input_has_no_address`. There is no test where the scriptSig's last push is not a valid P2PKH-shaped pubkey — for example a P2SH spend with a redeem-script push, a 3+-push scriptSig, or a 33/65-byte push that fails `PublicKey::from_slice`. These are exactly the ambiguous cases the length + parse checks are meant to filter out, and they'd pin the intended P2PKH-only contract so a future loosening of the heuristic (or the fix suggested above) has regression coverage.
In `packages/rs-platform-wallet-ffi/src/tx_decode.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/tx_decode.rs:87-100: `*out_decoded` is not zeroed on failure paths
On every error return (`check_ptr!(tx_bytes)`, the `tx_bytes_len == 0` branch, `unwrap_result_or_return!(decode_transaction(...))`), the function returns a non-Success `PlatformWalletFFIResult` without assigning to `*out_decoded`. The Swift wrapper is safe because it initializes `outDecoded = nil` and checks the result code first, but any other C/Swift caller that reuses an already-populated out pointer — or reads it before checking the result code — will observe stale data and may double-free by re-calling `platform_wallet_decoded_transaction_free` on the previous value. Convention for out-pointers is to null on entry: after `check_ptr!(out_decoded)`, do `*out_decoded = std::ptr::null_mut();`. Small change, removes a foot-gun for anyone consuming this from C.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/tx_decode.rs:30-34: Document that `DecodedTxInputFFI::address` is unauthenticated and must not be used for authorization
The per-input `address` is derived from the last scriptSig push with no signature verification tying it to the UTXO being spent. A crafter can put any 33/65-byte value in that position, so the field is trivially spoofable. The pure module doc says "best-effort / sent from", but the FFI struct comment only says the value is "recovered from a P2PKH scriptSig", which a downstream consumer (dashwallet-ios, future CrowdNode integration) could reasonably read as "the sender is P" and use for routing/authorization decisions. Strengthen the comment on `DecodedTxInputFFI::address` (and the mirrored Swift `Input.address`) to say explicitly: derived from an unauthenticated script push, safe for display or matching hints only, not for authentication/authorization. The safe matching primitive for CrowdNode-style protocols is per-output `(address, amount)`, which is already exposed and not attacker-influenceable in the same way.
| fn input_address_from_script_sig(script_sig: &ScriptBuf, network: Network) -> Option<Address> { | ||
| let mut last_push: Option<Vec<u8>> = None; | ||
| for instruction in script_sig.instructions() { | ||
| match instruction { | ||
| Ok(Instruction::PushBytes(bytes)) => last_push = Some(bytes.as_bytes().to_vec()), | ||
| Ok(_) => {} | ||
| Err(_) => return None, | ||
| } | ||
| } | ||
| let candidate = last_push?; | ||
| if candidate.len() != 33 && candidate.len() != 65 { | ||
| return None; | ||
| } | ||
| let pubkey = PublicKey::from_slice(&candidate).ok()?; | ||
| Some(Address::p2pkh(&pubkey, network)) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: P2PKH sender heuristic doesn't verify scriptSig shape, only the last push's length
input_address_from_script_sig iterates every instruction in the scriptSig and takes whichever data push occurred last, then treats it as a pubkey if it is 33 or 65 bytes. It never checks that the scriptSig actually has the P2PKH shape (exactly two pushes: signature then pubkey). For a P2SH spend the last push is the redeem script; if that redeem script happens to be 33 or 65 bytes and parses as a valid secp256k1 point (spender-controllable, only constrained by hashing to the P2SH address), the function silently returns a P2PKH address unrelated to the actual signer. The module doc claims None for non-P2PKH, but the implementation doesn't uphold that.
The simplest tightening is to require exactly two pushes (sig, pubkey) — anything else, including multisig P2SH redeem scripts, returns None. An additional guard that the first push looks like a DER-encoded ECDSA signature (starts with 0x30, length ≤ 73) would eliminate essentially all remaining collision risk. This is not blocking for the stated CrowdNode use case (which matches on outputs, not input senders), but the failure mode — a wrong non-null address is worse than None — is worth closing since callers building sender-verification logic on top of input.address would be fooled.
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — P2PKH sender heuristic doesn't verify scriptSig shape, only the last push's length no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| let outputs = tx | ||
| .output | ||
| .iter() | ||
| .map(|txout| DecodedOutput { | ||
| address: Address::from_script(&txout.script_pubkey, network).ok(), | ||
| value_duffs: txout.value, | ||
| script_pubkey: txout.script_pubkey.clone(), | ||
| }) | ||
| .collect(); | ||
|
|
||
| Ok(DecodedTransaction { | ||
| txid: tx.txid(), | ||
| inputs, | ||
| outputs, | ||
| }) |
There was a problem hiding this comment.
🟡 Suggestion: Unnecessary clone of every output's scriptPubkey
decode_transaction iterates tx.output by reference and clones every script_pubkey to populate DecodedOutput::script_pubkey. Since tx.txid() only needs &self and doesn't care about the order relative to consuming other fields, the txid can be computed first, after which tx.output (and tx.input) can be consumed via into_iter() — eliminating the per-output ScriptBuf clone entirely. For transactions with large scripts (OP_RETURN payloads, multisig redeem scripts) this is a real, avoidable allocation on every decode call, and this function is positioned as the primitive for scanning arbitrary persisted transactions.
| let outputs = tx | |
| .output | |
| .iter() | |
| .map(|txout| DecodedOutput { | |
| address: Address::from_script(&txout.script_pubkey, network).ok(), | |
| value_duffs: txout.value, | |
| script_pubkey: txout.script_pubkey.clone(), | |
| }) | |
| .collect(); | |
| Ok(DecodedTransaction { | |
| txid: tx.txid(), | |
| inputs, | |
| outputs, | |
| }) | |
| let txid = tx.txid(); | |
| let inputs = tx | |
| .input | |
| .into_iter() | |
| .map(|txin| { | |
| let address = if txin.previous_output.is_null() { | |
| None // coinbase | |
| } else { | |
| input_address_from_script_sig(&txin.script_sig, network) | |
| }; | |
| DecodedInput { | |
| previous_output: txin.previous_output, | |
| address, | |
| } | |
| }) | |
| .collect(); | |
| let outputs = tx | |
| .output | |
| .into_iter() | |
| .map(|txout| DecodedOutput { | |
| address: Address::from_script(&txout.script_pubkey, network).ok(), | |
| value_duffs: txout.value, | |
| script_pubkey: txout.script_pubkey, | |
| }) | |
| .collect(); | |
| Ok(DecodedTransaction { | |
| txid, | |
| inputs, | |
| outputs, | |
| }) |
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — Unnecessary clone of every output's scriptPubkey no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| pub unsafe extern "C" fn platform_wallet_decode_transaction( | ||
| tx_bytes: *const u8, | ||
| tx_bytes_len: usize, | ||
| network: FFINetwork, | ||
| out_decoded: *mut *mut DecodedTransactionFFI, | ||
| ) -> PlatformWalletFFIResult { | ||
| check_ptr!(tx_bytes); | ||
| check_ptr!(out_decoded); | ||
| if tx_bytes_len == 0 { | ||
| return PlatformWalletFFIResult::err( | ||
| PlatformWalletFFIResultCode::ErrorInvalidParameter, | ||
| "tx_bytes_len is zero", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: *out_decoded is not zeroed on failure paths
On every error return (check_ptr!(tx_bytes), the tx_bytes_len == 0 branch, unwrap_result_or_return!(decode_transaction(...))), the function returns a non-Success PlatformWalletFFIResult without assigning to *out_decoded. The Swift wrapper is safe because it initializes outDecoded = nil and checks the result code first, but any other C/Swift caller that reuses an already-populated out pointer — or reads it before checking the result code — will observe stale data and may double-free by re-calling platform_wallet_decoded_transaction_free on the previous value. Convention for out-pointers is to null on entry: after check_ptr!(out_decoded), do *out_decoded = std::ptr::null_mut();. Small change, removes a foot-gun for anyone consuming this from C.
source: ['claude']
There was a problem hiding this comment.
Correction: this broad out-param thread is not fully resolved in this update. The issue has been narrowed: most failure paths now clear *out_decoded, but the tx_bytes == NULL path still returns before the documented "nulled on entry" invariant is applied.
I carried the narrowed finding forward in the exact-SHA review for 225c1ae3: #3981 (review)
There was a problem hiding this comment.
The remaining null-tx_bytes gap is closed in ffa8c94 — the out-param is now nulled before any input validation, and the pre-poison test covers that path too.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use dashcore::consensus::serialize; | ||
| use dashcore::hashes::Hash; | ||
| use dashcore::secp256k1::{Secp256k1, SecretKey}; | ||
| use dashcore::{TxIn, TxOut, Witness}; | ||
|
|
||
| fn test_pubkey() -> PublicKey { | ||
| let secp = Secp256k1::new(); | ||
| let sk = SecretKey::from_slice(&[0x42u8; 32]).expect("valid secret key"); | ||
| PublicKey::new(sk.public_key(&secp)) | ||
| } | ||
|
|
||
| /// One P2PKH input (spending 11..11:3), one P2PKH output of 151072 duffs, | ||
| /// one OP_RETURN output. The 151072 amount mirrors a CrowdNode signUp | ||
| /// signal so the fixture doubles as documentation. | ||
| fn synthetic_tx(network: Network) -> (Transaction, Address) { | ||
| let pubkey = test_pubkey(); | ||
| let addr = Address::p2pkh(&pubkey, network); | ||
| let script_sig = dashcore::blockdata::script::Builder::new() | ||
| .push_slice(&[0x30u8; 71]) | ||
| .push_key(&pubkey) | ||
| .into_script(); | ||
| let tx = Transaction { | ||
| version: 1, | ||
| lock_time: 0, | ||
| input: vec![TxIn { | ||
| previous_output: OutPoint { | ||
| txid: Txid::from_byte_array([0x11u8; 32]), | ||
| vout: 3, | ||
| }, | ||
| script_sig, | ||
| sequence: 0xffffffff, | ||
| witness: Witness::default(), | ||
| }], | ||
| output: vec![ | ||
| TxOut { | ||
| value: 151_072, | ||
| script_pubkey: addr.script_pubkey(), | ||
| }, | ||
| TxOut { | ||
| value: 0, | ||
| script_pubkey: ScriptBuf::new_op_return(&[0xAAu8; 4]), | ||
| }, | ||
| ], | ||
| special_transaction_payload: None, | ||
| }; | ||
| (tx, addr) | ||
| } | ||
|
|
||
| #[test] | ||
| fn decodes_outputs_with_addresses_and_values() { | ||
| let (tx, addr) = synthetic_tx(Network::Testnet); | ||
| let decoded = decode_transaction(&serialize(&tx), Network::Testnet).unwrap(); | ||
|
|
||
| assert_eq!(decoded.txid, tx.txid()); | ||
| assert_eq!(decoded.outputs.len(), 2); | ||
|
|
||
| assert_eq!(decoded.outputs[0].address.as_ref(), Some(&addr)); | ||
| assert_eq!(decoded.outputs[0].value_duffs, 151_072); | ||
| assert_eq!(decoded.outputs[0].script_pubkey, addr.script_pubkey()); | ||
| assert!( | ||
| addr.to_string().starts_with('y'), | ||
| "testnet P2PKH starts with 'y'" | ||
| ); | ||
|
|
||
| // OP_RETURN: no address, script still present. | ||
| assert!(decoded.outputs[1].address.is_none()); | ||
| assert!(!decoded.outputs[1].script_pubkey.as_bytes().is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn recovers_p2pkh_input_address_from_script_sig() { | ||
| let (tx, addr) = synthetic_tx(Network::Testnet); | ||
| let decoded = decode_transaction(&serialize(&tx), Network::Testnet).unwrap(); | ||
|
|
||
| assert_eq!(decoded.inputs.len(), 1); | ||
| assert_eq!( | ||
| decoded.inputs[0].previous_output.txid, | ||
| Txid::from_byte_array([0x11u8; 32]) | ||
| ); | ||
| assert_eq!(decoded.inputs[0].previous_output.vout, 3); | ||
| assert_eq!(decoded.inputs[0].address.as_ref(), Some(&addr)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn network_changes_rendered_addresses() { | ||
| let (tx, testnet_addr) = synthetic_tx(Network::Testnet); | ||
| let decoded = decode_transaction(&serialize(&tx), Network::Mainnet).unwrap(); | ||
| let rendered = decoded.outputs[0].address.as_ref().unwrap().to_string(); | ||
| assert_ne!(rendered, testnet_addr.to_string()); | ||
| assert!(rendered.starts_with('X'), "mainnet P2PKH starts with 'X'"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn coinbase_input_has_no_address() { | ||
| let (mut tx, _) = synthetic_tx(Network::Testnet); | ||
| tx.input[0].previous_output = OutPoint::null(); | ||
| tx.input[0].script_sig = ScriptBuf::new(); | ||
| let decoded = decode_transaction(&serialize(&tx), Network::Testnet).unwrap(); | ||
| assert!(decoded.inputs[0].address.is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn trailing_garbage_is_an_error() { | ||
| let (tx, _) = synthetic_tx(Network::Testnet); | ||
| let mut bytes = serialize(&tx); | ||
| bytes.extend_from_slice(&[0xDE, 0xAD]); | ||
| assert!(decode_transaction(&bytes, Network::Testnet).is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn garbage_bytes_are_an_error() { | ||
| assert!(decode_transaction(&[0xFFu8; 16], Network::Testnet).is_err()); | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: No test exercises a non-P2PKH scriptSig (the documented None path)
The only "no address" input test is coinbase_input_has_no_address. There is no test where the scriptSig's last push is not a valid P2PKH-shaped pubkey — for example a P2SH spend with a redeem-script push, a 3+-push scriptSig, or a 33/65-byte push that fails PublicKey::from_slice. These are exactly the ambiguous cases the length + parse checks are meant to filter out, and they'd pin the intended P2PKH-only contract so a future loosening of the heuristic (or the fix suggested above) has regression coverage.
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — No test exercises a non-P2PKH scriptSig (the documented None path) no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| /// Sender address recovered from a P2PKH scriptSig, or null when the | ||
| /// input is coinbase / non-P2PKH / unparseable. Owned by the parent | ||
| /// structure — freed by `platform_wallet_decoded_transaction_free`. | ||
| pub address: *mut c_char, | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Document that DecodedTxInputFFI::address is unauthenticated and must not be used for authorization
The per-input address is derived from the last scriptSig push with no signature verification tying it to the UTXO being spent. A crafter can put any 33/65-byte value in that position, so the field is trivially spoofable. The pure module doc says "best-effort / sent from", but the FFI struct comment only says the value is "recovered from a P2PKH scriptSig", which a downstream consumer (dashwallet-ios, future CrowdNode integration) could reasonably read as "the sender is P" and use for routing/authorization decisions. Strengthen the comment on DecodedTxInputFFI::address (and the mirrored Swift Input.address) to say explicitly: derived from an unauthenticated script push, safe for display or matching hints only, not for authentication/authorization. The safe matching primitive for CrowdNode-style protocols is per-output (address, amount), which is already exposed and not attacker-influenceable in the same way.
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — Document that DecodedTxInputFFI::address is unauthenticated and must not be used for authorization no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| fn input_address_from_script_sig(script_sig: &ScriptBuf, network: Network) -> Option<Address> { | ||
| let mut last_push: Option<Vec<u8>> = None; | ||
| for instruction in script_sig.instructions() { | ||
| match instruction { | ||
| Ok(Instruction::PushBytes(bytes)) => last_push = Some(bytes.as_bytes().to_vec()), | ||
| Ok(_) => {} | ||
| Err(_) => return None, | ||
| } | ||
| } | ||
| let candidate = last_push?; | ||
| if candidate.len() != 33 && candidate.len() != 65 { | ||
| return None; | ||
| } | ||
| let pubkey = PublicKey::from_slice(&candidate).ok()?; | ||
| Some(Address::p2pkh(&pubkey, network)) | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: last_push reallocates on every push instead of holding a borrow
The loop allocates a fresh Vec<u8> for every push instruction, discarding the previous allocation, only to keep the last one. Since PushBytes borrows from the underlying ScriptBuf, you can hold &[u8] for the loop duration and only inspect it at the end. Not a hot-path concern, but easy to fix and avoids O(n_pushes) allocations.
| fn input_address_from_script_sig(script_sig: &ScriptBuf, network: Network) -> Option<Address> { | |
| let mut last_push: Option<Vec<u8>> = None; | |
| for instruction in script_sig.instructions() { | |
| match instruction { | |
| Ok(Instruction::PushBytes(bytes)) => last_push = Some(bytes.as_bytes().to_vec()), | |
| Ok(_) => {} | |
| Err(_) => return None, | |
| } | |
| } | |
| let candidate = last_push?; | |
| if candidate.len() != 33 && candidate.len() != 65 { | |
| return None; | |
| } | |
| let pubkey = PublicKey::from_slice(&candidate).ok()?; | |
| Some(Address::p2pkh(&pubkey, network)) | |
| } | |
| fn input_address_from_script_sig(script_sig: &ScriptBuf, network: Network) -> Option<Address> { | |
| let mut last_push: Option<&[u8]> = None; | |
| for instruction in script_sig.instructions() { | |
| match instruction { | |
| Ok(Instruction::PushBytes(bytes)) => last_push = Some(bytes.as_bytes()), | |
| Ok(_) => {} | |
| Err(_) => return None, | |
| } | |
| } | |
| let candidate = last_push?; | |
| if candidate.len() != 33 && candidate.len() != 65 { | |
| return None; | |
| } | |
| let pubkey = PublicKey::from_slice(candidate).ok()?; | |
| Some(Address::p2pkh(&pubkey, network)) | |
| } |
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — last_push reallocates on every push instead of holding a borrow no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| public struct DecodedTransaction: Sendable, Equatable { | ||
| public struct Input: Sendable, Equatable { | ||
| /// Previous output's txid in consensus (internal) byte order. | ||
| public let prevTxid: Data | ||
| /// Previous output's index. | ||
| public let prevVout: UInt32 | ||
| /// Sender address recovered from a P2PKH scriptSig; nil for | ||
| /// coinbase / non-P2PKH / unparseable script sigs. | ||
| public let address: String? | ||
| } | ||
|
|
||
| public struct Output: Sendable, Equatable { | ||
| /// Destination address for standard scripts (P2PKH / P2SH); nil for | ||
| /// non-standard scripts (OP_RETURN, bare multisig, …). | ||
| public let address: String? | ||
| /// Output value in duffs. | ||
| public let valueDuffs: UInt64 | ||
| /// Raw scriptPubKey bytes. | ||
| public let scriptPubkey: Data | ||
| } | ||
|
|
||
| /// Transaction id in consensus (internal) byte order — reverse for | ||
| /// explorer-style display (see `txidDisplayHex`). | ||
| public let txid: Data | ||
| public let inputs: [Input] | ||
| public let outputs: [Output] | ||
|
|
||
| /// Explorer-style (reversed, hex) rendering of `txid`. | ||
| public var txidDisplayHex: String { | ||
| txid.reversed().map { String(format: "%02x", $0) }.joined() | ||
| } | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: Input.prevTxid lacks a display-hex helper analogous to txidDisplayHex
txid gets a txidDisplayHex computed property that reverses to explorer order, but Input.prevTxid (same consensus/internal byte-order convention per the FFI doc comment) has no parallel helper. Callers matching a stored transaction's inputs against an explorer-copied txid string will silently produce a byte-reversed mismatch. Given the CrowdNode use case involves matching known txids, exposing a symmetric prevTxidDisplayHex (or a shared helper) would prevent a common footgun. Pure ergonomics/consistency note.
source: ['claude']
There was a problem hiding this comment.
Resolved in this update — Input.prevTxid lacks a display-hex helper analogous to txidDisplayHex no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #3981 +/- ##
============================================
+ Coverage 87.19% 87.21% +0.02%
============================================
Files 2636 2642 +6
Lines 327822 328272 +450
============================================
+ Hits 285852 286315 +463
+ Misses 41970 41957 -13
🚀 New features to boost your workflow:
|
|
Opened a focused helper PR for the CodeRabbit test feedback: #3993 It removes the tautological txid assertion and pins the expected |
|
|
||
| /// A consensus-decoded transaction with addresses rendered for a network. | ||
| #[derive(Debug, Clone)] | ||
| pub struct DecodedTransaction { |
There was a problem hiding this comment.
Why do you need this step?? Isn't decoding bytes into a Transaction and then transforming it into the FFI structure more than enough?? I don't see a usage for DecodedTransaction/DecodedInput/DecodedOutput
There was a problem hiding this comment.
Agreed — done in 4eaa825. platform_wallet_decode_transaction now deserializes straight into dashcore::Transaction and marshals that into the FFI structs; the transaction_decode module and its intermediate types are gone. The FFI struct layout and symbols are unchanged, so the generated header and the Swift wrapper API are unaffected.
| /// One P2PKH input (spending 11..11:3), one P2PKH output of 151072 duffs, | ||
| /// one OP_RETURN output. The 151072 amount mirrors a CrowdNode signUp | ||
| /// signal so the fixture doubles as documentation. | ||
| fn synthetic_tx(network: Network) -> (Transaction, Address) { |
There was a problem hiding this comment.
If you need Transaction for your tests there is a test_utils module in the dashcore crate, it may fit your needs, and if it doesn't let me now so I can consider updating the API
There was a problem hiding this comment.
Adopted in 4eaa825 — the tests now use Address::dummy, Transaction::dummy, and Transaction::dummy_coinbase (via a dashcore = { workspace = true, features = ["test-utils"] } dev-dependency). One gap: there's no builder for a P2PKH-shaped scriptSig (signature push + pubkey push) — Transaction::dummy fills script_sig with the lock script — so the fixture for the sender-address recovery path stays hand-built. If you'd like to add something like Transaction::dummy_p2pkh_spend to test_utils I'd happily consume it, but it's not blocking.
| /// One decoded transaction input. | ||
| #[repr(C)] | ||
| pub struct DecodedTxInputFFI { | ||
| /// Previous output's txid in consensus (internal) byte order. |
There was a problem hiding this comment.
Instead of creating new structs and mapping every field, have you consider an opaque pointer like:
#[repr(C)]
pub struct FFITransaction {
inner: *mut c_void, // This is a pointer to the heap allocated transaction decoded
}Then, instead of reading fields, we can call struct methods exposed though the FFI that are able to convert that inner pointer into the real transaction, and get the data you need. This may not be ideal, feel free to say no
| /// One decoded transaction input. | ||
| #[repr(C)] | ||
| pub struct DecodedTxInputFFI { | ||
| /// Previous output's txid in consensus (internal) byte order. |
There was a problem hiding this comment.
We can maybe update the places where the transaction bytes are being returned to return this FFI structure directly, avoiding having to decode it.
There was a problem hiding this comment.
Good idea — that would save those consumers the decode round-trip. I'd keep it out of this PR though, since it changes the signatures of existing byte-returning APIs and their consumers. Follow-up material?
…structs Review response for #3981: - drop the intermediate DecodedTransaction/DecodedInput/DecodedOutput types and the transaction_decode module in rs-platform-wallet; the FFI entry point now consensus-decodes into dashcore::Transaction and marshals it straight into the (unchanged) C structures - tighten the P2PKH sender heuristic: exactly two data pushes (DER-shaped signature, then 33/65-byte pubkey) and nothing else, so P2SH redeem scripts no longer masquerade as sender addresses - null *out_decoded on entry so failure paths can't leak stale pointers to C callers - consume the decoded tx (into_iter + ScriptBuf::into_bytes) instead of cloning every scriptPubkey; hold borrows in the heuristic instead of reallocating per push - document input.address as unauthenticated (display/matching only) on both the FFI struct and the Swift mirror; add prevTxidDisplayHex - move all tests to the FFI crate, through the public entry point; use dashcore test_utils fixtures where they fit (Address::dummy, Transaction::dummy, dummy_coinbase) and pin the new heuristic rejections (non-P2PKH scriptSig, redeem-script collision, non-DER first push) plus null-on-failure FFI surface (struct layouts, symbols) is byte-identical to the previous commit, so the generated header and Swift wrapper API are unaffected.
|
Review-bot suggestions applied in 4eaa825 alongside the restructure: the P2PKH sender heuristic now requires exactly two pushes (DER-shaped signature + 33/65-byte pubkey, anything else → null) so redeem scripts can't masquerade as sender addresses; |
There was a problem hiding this comment.
Code Review
Cumulative review from 4eaa8257 to 225c1ae3. The tx-decode FFI surface is unchanged since the prior review; the latest delta is a merge from origin/v4.1-dev containing unrelated work. The single carried-forward finding remains valid, and I found no new in-scope defects in the latest delta.
Source: reviewers = claude general opus (ok); codex general gpt-5.5 (ok, parsed after rc=5); claude security-auditor opus (ok); codex security-auditor gpt-5.5 (ok); claude ffi-engineer opus (ok); codex ffi-engineer gpt-5.5 (ok); verifier = claude opus (ok); specialists = security-auditor, ffi-engineer; policy gate = review_policy.enforce_backport_prereq_policy.
🟡 1 suggestion(s)
Prior Reconciliation
prior-1from4eaa8257: STILL VALID. At225c1ae3,packages/rs-platform-wallet-ffi/src/tx_decode.rs:142-144still checkstx_bytesbefore validating and clearingout_decoded, while the safety docs still promiseout_decodedis nulled on entry.
Carried-Forward Prior Findings
- [SUGGESTION]
packages/rs-platform-wallet-ffi/src/tx_decode.rs:142-144:*out_decodednot nulled on thetx_bytes == NULLpath, contradicting the documented safety contract
The rustdoc promises callers that the out-slot is cleared on entry, butcheck_ptr!(tx_bytes)returns before*out_decoded = null_mut()runs. A C/FFI caller that reuses an out-slot can therefore observe a stale pointer afterErrorNullPointer. The Swift wrapper initializes its local out pointer, so the current Swift path is not exposed; the risk is on the public C ABI surface. Validate and clearout_decodedbefore checkingtx_bytes, and extend the pre-poison test pattern to cover the null-tx_bytescase.
New Findings In Latest Delta
- None.
git diff 4eaa8257..225c1ae3is the upstream merge delta and does not touch the tx-decode FFI surface.
🤖 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/rs-platform-wallet-ffi/src/tx_decode.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/tx_decode.rs:142-144: `*out_decoded` not nulled on the tx_bytes-null path, contradicting documented safety contract
The rustdoc on `platform_wallet_decode_transaction` (lines 132-134) promises that `out_decoded` "is nulled on entry", so callers can trust the out-slot is cleared before any failure path. In the current ordering, `check_ptr!(tx_bytes)` returns early via the macro before line 144 clears `*out_decoded`. A C/FFI caller that reuses an out-slot containing a previously-decoded pointer and then invokes this function with `tx_bytes == NULL` receives `ErrorNullPointer` while the stale pointer remains visible. The fix is to validate and clear `out_decoded` first, then validate `tx_bytes`.
| check_ptr!(tx_bytes); | ||
| check_ptr!(out_decoded); | ||
| *out_decoded = std::ptr::null_mut(); |
There was a problem hiding this comment.
🟡 Suggestion: *out_decoded not nulled on the tx_bytes-null path, contradicting documented safety contract
The rustdoc on platform_wallet_decode_transaction (lines 132-134) promises that out_decoded "is nulled on entry", so callers can trust the out-slot is cleared before any failure path. In the current ordering, check_ptr!(tx_bytes) (line 142) returns early via the macro before line 144 clears *out_decoded. A C/FFI caller that reuses an out-slot containing a previously-decoded (and possibly already-freed) DecodedTransactionFFI* and then invokes this function with tx_bytes == NULL receives ErrorNullPointer while the stale pointer remains visible. Contract-following cleanup code that calls platform_wallet_decoded_transaction_free(*out) on the assumption the slot was cleared then double-frees or dereferences freed memory. The Swift wrapper (TransactionDecoder.decode) zero-initializes its local outDecoded, so it is not exposed today; the risk lives on the public C ABI surface. The existing rejects_null_and_empty_input test never asserts the out-param state after the null-tx_bytes case, which is why the mismatch slipped through — extending the pre-poison pattern used by trailing_garbage_is_a_deserialization_error would pin the fix. The fix is one line: validate and clear out_decoded first, then validate tx_bytes.
| check_ptr!(tx_bytes); | |
| check_ptr!(out_decoded); | |
| *out_decoded = std::ptr::null_mut(); | |
| check_ptr!(out_decoded); | |
| *out_decoded = std::ptr::null_mut(); | |
| check_ptr!(tx_bytes); |
source: ['claude', 'codex']
There was a problem hiding this comment.
Fixed in ffa8c94 — out_decoded is now validated and nulled before check_ptr!(tx_bytes), so the documented "nulled on entry" contract holds on every failure path. rejects_null_and_empty_input now pre-poisons the out param (same pattern as trailing_garbage_is_a_deserialization_error) and asserts it comes back null for both the null-tx_bytes and zero-length cases.
There was a problem hiding this comment.
Resolved in ffa8c94 — *out_decoded not nulled on the tx_bytes-null path, contradicting documented safety contract no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
The safety docs promise out_decoded is nulled on entry, but check_ptr!(tx_bytes) returned first, so a NULL tx_bytes left a stale pointer visible in the out-slot on the public C ABI. Reorder the checks and pre-poison the out param in rejects_null_and_empty_input to pin both failure paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Cumulative review from 225c1ae3 to ffa8c943. The latest delta directly fixes the prior FFI out-param contract issue: platform_wallet_decode_transaction now validates and clears out_decoded before checking tx_bytes, and rejects_null_and_empty_input pre-poisons the out slot and asserts it is nulled on both null-tx_bytes and zero-length failure paths. No carried-forward prior findings remain, and I found no new in-scope defects in the latest delta or current PR head.
Source: reviewers = claude general opus (ok); codex general gpt-5.5 (ok, parsed after rc=5); claude security-auditor opus (ok); codex security-auditor gpt-5.5 (ok); claude ffi-engineer opus (ok); codex ffi-engineer gpt-5.5 (ok); verifier = claude opus (ok); specialists = security-auditor, ffi-engineer; policy gate = review_policy.enforce_backport_prereq_policy.
Prior Reconciliation:
prior-1from225c1ae3: FIXED. Atffa8c943,packages/rs-platform-wallet-ffi/src/tx_decode.rs:142-144now runscheck_ptr!(out_decoded); *out_decoded = std::ptr::null_mut(); check_ptr!(tx_bytes);, so the documented "nulled on entry" contract holds even whentx_bytes == NULL. The regression test now pre-poisons the out pointer and asserts it is nulled for the null and empty-input failure paths.
Carried-Forward Prior Findings:
- None.
New Findings In Latest Delta:
- None.
git diff 225c1ae3..ffa8c943only reorders the FFI pointer checks and strengthens the null/empty-input test around the prior finding.
3 participants
Issue being fixed or feature implemented
Host apps consuming the platform-wallet FFI (dashwallet-ios mid-migration off DashSync) need to match arbitrary persisted transactions against external addresses and exact amounts — e.g. the CrowdNode on-chain API protocol: "did a transaction pay
apiOffset + codeduffs to the CrowdNode address?" and "did CrowdNode reply with amount X to my account address?".The wallet layer persists every transaction's raw bytes, but only materializes TXO rows for the wallet's own outputs, and
AccountTransactionEntryFFIcarries no per-output data. There is currently no way to evaluate a per-output(address, amount)predicate from either the FFI or the Swift SDK — payments to external addresses (CrowdNode's) have no addressable representation at all. This blocks porting CrowdNode's on-chain response tracking (and several tx-history readers) off DashSync.What was done?
Adds a stateless transaction-decode FFI entry point with a Swift wrapper. (Reworked per review: the initial version had an intermediate pure-crate representation in
rs-platform-wallet; that layer is gone — the FFI decodes straight intodashcore::Transactionand maps it directly into the C structs.)rs-platform-wallet-ffi—src/tx_decode.rs:platform_wallet_decode_transaction(tx_bytes, len, network, out) -> PlatformWalletFFIResult— consensus-decodes raw tx bytes intodashcore::Transaction(rejecting trailing bytes) and marshals it into#[repr(C)]structs:txid(consensus byte order),addressviaAddress::from_script(null for non-standard scripts like OP_RETURN),value_duffs, rawscript_pubkeybytes,address, recovered only for strictly P2PKH-shaped scriptSigs (exactly two pushes: DER-shaped signature, then 33/65-byte pubkey — anything else, incl. P2SH redeem scripts, yields null). Documented as unauthenticated: display/matching hint only, never authorization; the attacker-resistant matching primitive is the per-output(address, value_duffs)pair.platform_wallet_decoded_transaction_free— paired free (null-safe);*out_decodedis nulled on entry so failure paths can't leak stale pointers to C callers.dashcore::test_utils(Address::dummy,Transaction::dummy,Transaction::dummy_coinbase) where they fit; the P2PKH-scriptSig fixture is hand-built (test_utils has no builder for that shape).swift-sdk—Sources/SwiftDashSDK/PlatformWallet/TransactionDecoder.swift:TransactionDecoder.decode(_ txData: Data, network: Network) throws -> DecodedTransaction— typed structs,PlatformWalletErrormapping,defer-freed FFI memory,txidDisplayHex/prevTxidDisplayHexdisplay helpers. Unit tests inTransactionDecoderTests(error-case assertions tightened separately in test(swift-sdk): assert transaction decoder error cases #3993).How Has This Been Tested?
cargo test -p platform-wallet -p platform-wallet-ffi— full suites green (202 + 112 tests).cargo fmt --check+cargo clippy --all-targetson both crates — clean.Breaking Changes
None — purely additive (new FFI symbols, new Swift wrapper).
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit