-
Notifications
You must be signed in to change notification settings - Fork 12
feat(dash-spv-ffi): fixed building and signing a transaction and implemented exposed broadcasting in the FFI #447
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -535,6 +535,21 @@ struct FFISyncProgress *dash_spv_ffi_client_get_manager_sync_progress(struct FFI | |
| */ | ||
| int32_t dash_spv_ffi_client_clear_storage(struct FFIDashSpvClient *client) ; | ||
|
|
||
| /** | ||
| * Broadcasts a transaction to the Dash network via connected peers. | ||
| * | ||
| * # Safety | ||
| * | ||
| * - `client` must be a valid, non-null pointer to an initialized FFIDashSpvClient | ||
| * - `tx_bytes` must be a valid, non-null pointer to the transaction data | ||
| * - `length` must be the length of the transaction data in bytes | ||
| */ | ||
|
|
||
| int32_t dash_spv_ffi_client_broadcast_transaction(struct FFIDashSpvClient *client, | ||
| const uint8_t *tx_bytes, | ||
| uintptr_t length) | ||
| ; | ||
|
Comment on lines
+548
to
+551
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check for test coverage of dash_spv_ffi_client_broadcast_transaction
echo "=== Searching for broadcast_transaction in Rust test files ==="
find . -type f -path "*/tests/unit/*" -name "*.rs" | xargs grep -l "broadcast_transaction" 2>/dev/null || echo "No matches in unit tests"
echo ""
echo "=== Searching for broadcast_transaction in C test files ==="
find . -type f -path "*/tests/c_tests/*" \( -name "*.c" -o -name "*.h" \) | xargs grep -l "broadcast_transaction" 2>/dev/null || echo "No matches in C tests"
echo ""
echo "=== All test directories in dash-spv-ffi ==="
find dash-spv-ffi/tests -type d 2>/dev/null | head -20
echo ""
echo "=== Checking specific test directories ==="
ls -la dash-spv-ffi/tests/unit/ 2>/dev/null || echo "unit directory not found"
ls -la dash-spv-ffi/tests/c_tests/ 2>/dev/null || echo "c_tests directory not found"Repository: dashpay/rust-dashcore Length of output: 1412 🏁 Script executed: #!/bin/bash
# Check if dash_spv_ffi_client_broadcast_transaction is a new function
# and verify it exists in the source code
echo "=== Searching for dash_spv_ffi_client_broadcast_transaction implementation ==="
rg -rn "dash_spv_ffi_client_broadcast_transaction" --type=rust dash-spv-ffi/src/
echo ""
echo "=== Checking for broader transaction-related tests ==="
rg -rn "broadcast" --type=rust dash-spv-ffi/tests/unit/ | head -20
echo ""
echo "=== List of existing test files with their content overview ==="
for f in dash-spv-ffi/tests/unit/*.rs; do
echo "File: $(basename $f)"
head -20 "$f" | grep -E "^(mod |#\[test\]|fn test_)" || echo " (examining...)"
doneRepository: dashpay/rust-dashcore Length of output: 573 🏁 Script executed: #!/bin/bash
# Get the actual implementation of dash_spv_ffi_client_broadcast_transaction
echo "=== Full context around broadcast_transaction in client.rs ==="
rg -B5 -A15 "broadcast_transaction" dash-spv-ffi/src/client.rs
echo ""
echo "=== Check what tests import or test client functions ==="
grep -r "client\|broadcast" dash-spv-ffi/tests/unit/ --include="*.rs" | head -30
echo ""
echo "=== Check C test files for transaction-related tests ==="
grep -r "broadcast\|transaction" dash-spv-ffi/tests/c_tests/ --include="*.c" | head -30Repository: dashpay/rust-dashcore Length of output: 5077 Add unit and C tests for This new FFI function is missing required test coverage. Per project guidelines, add:
Tests should cover valid transaction broadcast, invalid transaction bytes, null pointer handling, and error cases. 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Destroy the client and free associated resources. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -481,6 +481,47 @@ pub unsafe extern "C" fn dash_spv_ffi_client_clear_storage(client: *mut FFIDashS | |
| } | ||
| } | ||
|
|
||
| /// Broadcasts a transaction to the Dash network via connected peers. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// - `client` must be a valid, non-null pointer to an initialized FFIDashSpvClient | ||
| /// - `tx_bytes` must be a valid, non-null pointer to the transaction data | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| /// - `length` must be the length of the transaction data in bytes | ||
| #[no_mangle] | ||
| pub unsafe extern "C" fn dash_spv_ffi_client_broadcast_transaction( | ||
| client: *mut FFIDashSpvClient, | ||
| tx_bytes: *const u8, | ||
| length: usize, | ||
| ) -> i32 { | ||
| null_check!(client); | ||
| null_check!(tx_bytes); | ||
|
|
||
| let tx_bytes = std::slice::from_raw_parts(tx_bytes, length); | ||
|
|
||
| let tx = match dashcore::consensus::deserialize::<dashcore::Transaction>(tx_bytes) { | ||
| Ok(t) => t, | ||
| Err(e) => { | ||
| set_last_error(&format!("Invalid transaction: {}", e)); | ||
| return FFIErrorCode::InvalidArgument as i32; | ||
| } | ||
| }; | ||
|
|
||
| let client = &(*client); | ||
|
|
||
| let spv_client = client.inner.clone(); | ||
|
|
||
| let result = client.runtime.block_on(async { spv_client.broadcast_transaction(&tx).await }); | ||
|
|
||
| match result { | ||
| Ok(_) => FFIErrorCode::Success as i32, | ||
| Err(e) => { | ||
| set_last_error(&format!("Failed to broadcast transaction: {}", e)); | ||
| FFIErrorCode::from(e) as i32 | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+492
to
+523
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for any tests related to broadcast_transaction in the FFI test directory
rg -rn "broadcast_transaction" dash-spv-ffi/tests/
# Also check the structure of unit tests
echo "=== Unit test files in dash-spv-ffi/tests/unit/ ==="
find dash-spv-ffi/tests/unit/ -name "*.rs" 2>/dev/null | head -20Repository: dashpay/rust-dashcore Length of output: 660 🏁 Script executed: #!/bin/bash
# Search for the exact function name in unit tests
rg -rn "dash_spv_ffi_client_broadcast_transaction" dash-spv-ffi/tests/unit/
# Check if it's referenced anywhere in tests
rg -rn "dash_spv_ffi_client_broadcast_transaction" dash-spv-ffi/tests/
# Examine the async operations unit test (most likely place for this function)
echo "=== Checking test_async_operations.rs ==="
wc -l dash-spv-ffi/tests/unit/test_async_operations.rs
# Also check if the function exists in the source
echo "=== Checking if dash_spv_ffi_client_broadcast_transaction exists in source ==="
rg -n "fn dash_spv_ffi_client_broadcast_transaction" dash-spv-ffi/src/Repository: dashpay/rust-dashcore Length of output: 551 🏁 Script executed: #!/bin/bash
# Read the test_async_operations.rs file to see what's covered
cat -n dash-spv-ffi/tests/unit/test_async_operations.rs | grep -i "broadcast\|test_\|fn " | head -40Repository: dashpay/rust-dashcore Length of output: 995 🏁 Script executed: #!/bin/bash
# Search for broadcast in all unit test files
grep -r "broadcast" dash-spv-ffi/tests/unit/ 2>/dev/null || echo "No broadcast found in unit tests"
# Check test_client_lifecycle.rs for relevant tests
echo "=== Checking test_client_lifecycle.rs for client operation tests ==="
cat -n dash-spv-ffi/tests/unit/test_client_lifecycle.rs | grep -i "fn test_"
# Check test_error_handling.rs
echo "=== Checking test_error_handling.rs ==="
cat -n dash-spv-ffi/tests/unit/test_error_handling.rs | grep -i "fn test_"Repository: dashpay/rust-dashcore Length of output: 1015 Add unit tests for Each new FFI function requires corresponding unit tests per project standards. Tests must cover the broadcast operation, error handling (invalid transaction, broadcast failures), and async behavior using the client's runtime. 🤖 Prompt for AI Agents |
||
|
|
||
| /// Destroy the client and free associated resources. | ||
| /// | ||
| /// # Safety | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ This document provides a comprehensive reference for all FFI (Foreign Function I | |
|
|
||
| **Auto-generated**: This documentation is automatically generated from the source code. Do not edit manually. | ||
|
|
||
| **Total Functions**: 261 | ||
| **Total Functions**: 259 | ||
|
|
||
| ## Table of Contents | ||
|
|
||
|
|
@@ -69,7 +69,7 @@ Functions: 19 | |
|
|
||
| ### Wallet Operations | ||
|
|
||
| Functions: 65 | ||
| Functions: 63 | ||
|
|
||
| | Function | Description | Module | | ||
| |----------|-------------|--------| | ||
|
|
@@ -109,7 +109,6 @@ Functions: 65 | |
| | `wallet_add_dashpay_receiving_account` | Add a DashPay receiving funds account # Safety - `wallet` must be a valid... | wallet | | ||
| | `wallet_add_platform_payment_account` | Add a Platform Payment account (DIP-17) to the wallet Platform Payment... | wallet | | ||
| | `wallet_build_and_sign_transaction` | Build and sign a transaction using the wallet's managed info This is the... | transaction | | ||
| | `wallet_build_transaction` | Build a transaction (unsigned) This creates an unsigned transaction | transaction | | ||
| | `wallet_check_transaction` | Check if a transaction belongs to the wallet using ManagedWalletInfo #... | transaction | | ||
| | `wallet_create_from_mnemonic` | Create a new wallet from mnemonic (backward compatibility - single network) ... | wallet | | ||
| | `wallet_create_from_mnemonic_with_options` | Create a new wallet from mnemonic with options # Safety - `mnemonic` must... | wallet | | ||
|
|
@@ -137,7 +136,6 @@ Functions: 65 | |
| | `wallet_get_xpub` | Get extended public key for account # Safety - `wallet` must be a valid... | wallet | | ||
| | `wallet_has_mnemonic` | Check if wallet has mnemonic # Safety - `wallet` must be a valid pointer... | wallet | | ||
| | `wallet_is_watch_only` | Check if wallet is watch-only # Safety - `wallet` must be a valid pointer... | wallet | | ||
| | `wallet_sign_transaction` | Sign a transaction # Safety - `wallet` must be a valid pointer to an... | transaction | | ||
|
|
||
| ### Account Management | ||
|
|
||
|
|
@@ -1286,30 +1284,14 @@ This function dereferences a raw pointer to FFIWallet. The caller must ensure th | |
| #### `wallet_build_and_sign_transaction` | ||
|
|
||
| ```c | ||
| wallet_build_and_sign_transaction(managed_wallet: *mut FFIManagedWalletInfo, wallet: *const FFIWallet, account_index: c_uint, outputs: *const FFITxOutput, outputs_count: usize, fee_per_kb: u64, current_height: u32, tx_bytes_out: *mut *mut u8, tx_len_out: *mut usize, error: *mut FFIError,) -> bool | ||
| wallet_build_and_sign_transaction(manager: *const FFIWalletManager, wallet: *const FFIWallet, account_index: u32, outputs: *const FFITxOutput, outputs_count: usize, fee_rate: FFIFeeRate, fee_out: *mut u64, tx_bytes_out: *mut *mut u8, tx_len_out: *mut usize, error: *mut FFIError,) -> bool | ||
| ``` | ||
|
|
||
| **Description:** | ||
| Build and sign a transaction using the wallet's managed info This is the recommended way to build transactions. It handles: - UTXO selection using coin selection algorithms - Fee calculation - Change address generation - Transaction signing # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `wallet` must be a valid pointer to an FFIWallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
| Build and sign a transaction using the wallet's managed info This is the recommended way to build transactions. It handles: - UTXO selection using coin selection algorithms - Fee calculation - Change address generation - Transaction signing # Safety - `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_rate` must be a valid variant of FFIFeeRate - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
| **Safety:** | ||
| - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `wallet` must be a valid pointer to an FFIWallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
| **Module:** `transaction` | ||
|
|
||
| --- | ||
|
|
||
| #### `wallet_build_transaction` | ||
|
|
||
| ```c | ||
| wallet_build_transaction(wallet: *mut FFIWallet, account_index: c_uint, outputs: *const FFITxOutput, outputs_count: usize, fee_per_kb: u64, tx_bytes_out: *mut *mut u8, tx_len_out: *mut usize, error: *mut FFIError,) -> bool | ||
| ``` | ||
|
|
||
| **Description:** | ||
| Build a transaction (unsigned) This creates an unsigned transaction. Use wallet_sign_transaction to sign it afterward. For a combined build+sign operation, use wallet_build_and_sign_transaction. # Safety - `wallet` must be a valid pointer to an FFIWallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
| **Safety:** | ||
| - `wallet` must be a valid pointer to an FFIWallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
| - `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_rate` must be a valid variant of FFIFeeRate - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
|
Comment on lines
+1287
to
1295
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The safety section states Since this doc is auto-generated, the fix belongs in the Rust source docstring for Additionally, the safety section does not specify the invariant that 📝 Suggested addition to the Rust source docstring /// - `fee_rate` must be a valid variant of FFIFeeRate
+/// (`FFIFeeRate::ECONOMY`, `FFIFeeRate::NORMAL`, or `FFIFeeRate::PRIORITY`)
+/// - `wallet` must be registered in `manager`; if the wallet ID is not found,
+/// `error` will be set and the function returns `false`🤖 Prompt for AI Agents |
||
| **Module:** `transaction` | ||
|
|
||
|
|
@@ -1747,22 +1729,6 @@ Check if wallet is watch-only # Safety - `wallet` must be a valid pointer to a | |
|
|
||
| --- | ||
|
|
||
| #### `wallet_sign_transaction` | ||
|
|
||
| ```c | ||
| wallet_sign_transaction(wallet: *const FFIWallet, tx_bytes: *const u8, tx_len: usize, signed_tx_out: *mut *mut u8, signed_len_out: *mut usize, error: *mut FFIError,) -> bool | ||
| ``` | ||
|
|
||
| **Description:** | ||
| Sign a transaction # Safety - `wallet` must be a valid pointer to an FFIWallet - `tx_bytes` must be a valid pointer to transaction bytes with at least `tx_len` bytes - `signed_tx_out` must be a valid pointer to store the signed transaction bytes pointer - `signed_len_out` must be a valid pointer to store the signed transaction length - `error` must be a valid pointer to an FFIError - The returned signed transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
| **Safety:** | ||
| - `wallet` must be a valid pointer to an FFIWallet - `tx_bytes` must be a valid pointer to transaction bytes with at least `tx_len` bytes - `signed_tx_out` must be a valid pointer to store the signed transaction bytes pointer - `signed_len_out` must be a valid pointer to store the signed transaction length - `error` must be a valid pointer to an FFIError - The returned signed transaction bytes must be freed with `transaction_bytes_free` | ||
|
|
||
| **Module:** `transaction` | ||
|
|
||
| --- | ||
|
|
||
| ### Account Management - Detailed | ||
|
|
||
| #### `account_collection_count` | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.