fix: ffi tari address from emoji#7114
Conversation
WalkthroughThe changes update internal logic for emoji string handling in address conversion. The size validation in the emoji-to-bytes helper is simplified, and error reporting is improved in the FFI conversion function. A new test is added to verify emoji string conversion via FFI. Additionally, three horizon sync tests are marked to be ignored due to incomplete prune mode functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant C_App as C Application
participant FFI as emoji_id_to_tari_address (FFI)
participant Rust as TariAddress
C_App->>FFI: Pass emoji C string pointer
FFI->>FFI: Convert C string pointer to Rust str
FFI->>Rust: TariAddress::from_emoji_string(str)
alt Success
Rust-->>FFI: TariAddress instance
FFI-->>C_App: Return pointer to address
else Error
Rust-->>FFI: Error
FFI->>FFI: Print error (dbg!)
FFI-->>C_App: Return null pointer, set error code
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
base_layer/wallet_ffi/src/lib.rs (1)
10928-10942: Strengthen the test by checking pointer validity and cleaning up memory.The new
test_emoji_stringleaks both theCStringand the heap-allocatedTariAddresspointer returned by the FFI call. It also only asserts the error code, not that the returned pointer is non-null. Consider:
- Asserting
result_ptris not null.- Reclaiming and dropping the raw
CStringviaCString::from_raw.- Reclaiming and dropping the
TariAddressviaBox::from_raw.Example patch:
let _result = emoji_id_to_tari_address(cstring_ptr, error_ptr); assert_eq!(*error_ptr, 0, "No error expected"); + assert!(!_result.is_null(), "Expected a valid TariAddress pointer"); + + // Clean up allocations + unsafe { + // reclaim and drop the input CString + CString::from_raw(cstring_ptr as *mut c_char); + // reclaim and drop the returned TariAddress + Box::from_raw(_result); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base_layer/common_types/src/tari_address/dual_address.rs(1 hunks)base_layer/common_types/src/tari_address/mod.rs(0 hunks)base_layer/wallet_ffi/src/lib.rs(2 hunks)
💤 Files with no reviewable changes (1)
- base_layer/common_types/src/tari_address/mod.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
base_layer/common_types/src/tari_address/dual_address.rs (2)
base_layer/common_types/src/tari_address/mod.rs (1)
from_emoji_string(270-274)base_layer/common_types/src/tari_address/single_address.rs (1)
from_emoji_string(93-96)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: Cucumber tests / FFI
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: cargo check with stable
Test Results (CI)363 tests 361 ✅ 8m 0s ⏱️ For more details on these failures, see this check. Results for commit 011941d. ♻️ This comment has been updated with latest results. |
Test Results (Integration tests)0 tests 0 ✅ 0s ⏱️ Results for commit cff9b1c. ♻️ This comment has been updated with latest results. |
SWvheerden
force-pushed
the
sw_fix_tari_address
branch
from
dd46dad to
87a8d43
Compare
2 participants
Description
Fixes the the tari address get address from emoji id
Summary by CodeRabbit
Bug Fixes
Tests
Chores