build: update rust-dashcore crates to v0.42-dev (542a617) #3104
Merged
Conversation
Update all 6 workspace dependency entries from rust-dashcore to the latest v0.42-dev commit (542a617843d1689217e386a678945caf461665c4), bumping 11 transitive crates from v0.41.0 to v0.42.0. Breaking changes addressed: - WalletBalance renamed to WalletCoreBalance - check_transaction renamed to check_core_transaction - ImmatureTransactionCollection removed; immature_transactions() now returns Vec<Transaction> - update_chain_height/add_immature_transaction/immature_balance/ process_matured_transactions removed from WalletInfoInterface - synced_height/update_synced_height added to WalletInfoInterface - dash_spv::bloom module removed; utility functions inlined in rs-dapi Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-dashcore-v042-dev
5 tasks
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated workspace git revisions for several dash crates; replaced external bloom utilities with internal script/outpoint handling in the DAPI streaming service; refactored platform-wallet public API, renaming transaction-check methods and changing balance, immature-transaction, and sync-height accessors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
lklimek
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
7-23: Consider named constants for P2PKH opcode bytes.The raw byte literals are correct but opaque; replacing them with constants improves readability and signals intent to future readers.
♻️ Proposed refactor
+const OP_DUP: u8 = 0x76; +const OP_HASH160: u8 = 0xa9; +const OP_PUSH_20: u8 = 0x14; // push 20 bytes +const OP_EQUALVERIFY: u8 = 0x88; +const OP_CHECKSIG: u8 = 0xac; fn extract_pubkey_hash(script: &Script) -> Option<Vec<u8>> { let bytes = script.as_bytes(); // P2PKH: OP_DUP OP_HASH160 <20 bytes> OP_EQUALVERIFY OP_CHECKSIG if bytes.len() == 25 - && bytes[0] == 0x76 // OP_DUP - && bytes[1] == 0xa9 // OP_HASH160 - && bytes[2] == 0x14 // Push 20 bytes - && bytes[23] == 0x88 // OP_EQUALVERIFY - && bytes[24] == 0xac + && bytes[0] == OP_DUP + && bytes[1] == OP_HASH160 + && bytes[2] == OP_PUSH_20 + && bytes[23] == OP_EQUALVERIFY + && bytes[24] == OP_CHECKSIG {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dapi/src/services/streaming_service/bloom.rs` around lines 7 - 23, Replace the opaque raw byte literals in extract_pubkey_hash with named u8 constants (e.g., OP_DUP, OP_HASH160, OP_EQUALVERIFY, OP_CHECKSIG, PUSH20) declared at module scope and use those constants in the bytes[] comparisons inside extract_pubkey_hash to make intent clear; define the constants near the top of the file (or reuse existing opcode definitions if available) and update the comment to reference the named constants instead of raw hex values.
25-31: Prefer.to_byte_array()over[..]indexing for stylistic consistency.
txid_to_be_bytes(line 51) usestxid.to_byte_array(). Using the same idiom inoutpoint_to_byteskeeps both helpers uniform; both expressions produce identical internal-LE bytes so there is no behavioral difference.♻️ Proposed refactor
fn outpoint_to_bytes(outpoint: &OutPoint) -> Vec<u8> { let mut bytes = Vec::with_capacity(36); - bytes.extend_from_slice(&outpoint.txid[..]); + bytes.extend_from_slice(&outpoint.txid.to_byte_array()); bytes.extend_from_slice(&outpoint.vout.to_le_bytes()); bytes }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dapi/src/services/streaming_service/bloom.rs` around lines 25 - 31, The outpoint_to_bytes helper uses &outpoint.txid[..] while txid_to_be_bytes uses txid.to_byte_array(); change outpoint_to_bytes to call outpoint.txid.to_byte_array() and extend_from_slice with that result to keep style consistent (still reserve capacity 36 and extend vout.to_le_bytes() as before), i.e., replace the slice indexing with a call to to_byte_array() inside outpoint_to_bytes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rs-dapi/src/services/streaming_service/bloom.rs`:
- Around line 7-23: Replace the opaque raw byte literals in extract_pubkey_hash
with named u8 constants (e.g., OP_DUP, OP_HASH160, OP_EQUALVERIFY, OP_CHECKSIG,
PUSH20) declared at module scope and use those constants in the bytes[]
comparisons inside extract_pubkey_hash to make intent clear; define the
constants near the top of the file (or reuse existing opcode definitions if
available) and update the comment to reference the named constants instead of
raw hex values.
- Around line 25-31: The outpoint_to_bytes helper uses &outpoint.txid[..] while
txid_to_be_bytes uses txid.to_byte_array(); change outpoint_to_bytes to call
outpoint.txid.to_byte_array() and extend_from_slice with that result to keep
style consistent (still reserve capacity 36 and extend vout.to_le_bytes() as
before), i.e., replace the slice indexing with a call to to_byte_array() inside
outpoint_to_bytes.
…of custom extract_pubkey_hash Replace hand-rolled `extract_pubkey_hash()` with `Script::p2pkh_public_key_hash_bytes()` and `Script::is_p2pkh()` from dashcore v0.42. This eliminates code duplication and avoids an unnecessary Vec allocation (the library method returns `&[u8]`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Failing Rust security audit test also fails on v3.1-dev |
Contributor
Author
|
Issues are pre-existing |
Contributor
Author
|
@copilot update this branch to use rust-dashcore a05d256f59743c69df912462dd77dd487e1ff5b2 |
Contributor
shumkov
requested changes
Feb 19, 2026
| use dashcore_rpc::dashcore::{OutPoint, ScriptBuf, Transaction as CoreTx, Txid}; | ||
|
|
||
| /// Convert outpoint to bytes for bloom filter | ||
| fn outpoint_to_bytes(outpoint: &OutPoint) -> Vec<u8> { |
Collaborator
There was a problem hiding this comment.
OutPoint doesn't have method to get bytes?
Contributor
Author
There was a problem hiding this comment.
good point, removed in favor of into()
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
…tom outpoint_to_bytes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-dapi/src/services/streaming_service/bloom.rs`:
- Around line 75-79: The txid byte-order is reversed incorrectly: update the
txid_to_be_bytes function to return the Txid in internal/wire (consensus) byte
order (i.e., use txid.to_byte_array().to_vec() and remove the arr.reverse()
step) so it matches OutPoint::into::<[u8;36]>() serialization and BIP37
expectations; also rename txid_to_be_bytes to a clearer name like
txid_to_internal_bytes (or txid_to_wire_bytes) and update the related unit test
that expected display/be (hex-reversed) bytes to expect the internal byte order
instead.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/rs-dapi/src/services/streaming_service/bloom.rs`:
- Around line 25-27: The txid byte-order bug was fixed by returning the
wire/internal order bytes in txid_to_internal_bytes; ensure the function
txid_to_internal_bytes returns txid.to_byte_array().to_vec() (no reversing) so
it matches BIP37 bloom expectations and OutPoint consensus encoding, and keep
that implementation as-is (add/update unit tests if needed to prevent
regressions).
shumkov
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Update all rust-dashcore workspace dependency crates to the latest v0.42-dev commit (
542a617843d1689217e386a678945caf461665c4).What was done?
dashcore,dash-spv,dash-spv-ffi,key-wallet,key-wallet-manager,dashcore-rpc) from53d699cto542a617, bumping 11 transitive crates from v0.41.0 to v0.42.0rs-platform-walletto upstream API changes:WalletBalance→WalletCoreBalancecheck_transaction→check_core_transactioninWalletTransactionCheckerimmature_transactions()now returnsVec<Transaction>(removedImmatureTransactionCollection)update_chain_height,add_immature_transaction,immature_balance,process_matured_transactionsfromWalletInfoInterfacesynced_heightandupdate_synced_heighttoWalletInfoInterfaceextract_pubkey_hashandoutpoint_to_bytesinrs-dapibloom module afterdash_spv::bloomwas removed upstreamHow Has This Been Tested?
cargo check --workspacepasses successfullycargo fmt --allappliedBreaking Changes
None — all changes are internal adaptations to upstream API changes.
Checklist:
Summary by CodeRabbit
Refactor
Chores
Tests