Merged
Conversation
…e (merge with 2.0.0-rc4, disable native tls), ferment is updated to 0.2.3
|
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 You can disable this status message by setting the 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request implements the new fermentize feature flag, enhances error handling and FFI conversions, updates document transition APIs to support an optional TokenPaymentInfo parameter, improves masternode processor logging, and makes miscellaneous build script and configuration improvements.
- Introduces and conditionally compiles fermentize functionality
- Enhances document state transition methods and error handling
- Refines masternode logging and cleans up build configuration
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dash-spv-platform/src/provider.rs | Adds a new API to retrieve token configuration with a placeholder error message |
| dash-spv-platform/src/lib.rs | Updates document transition tuples to include an optional TokenPaymentInfo parameter |
| dash-spv-masternode-processor/src/processing/processor/mod.rs | Adds detailed logging of quorum information using string concatenation |
| dash-spv-coinjoin/src/wallet_ex.rs | Removes an unnecessary clone on outpoint usage |
| dash-spv-apple-bindings/* | Improves build scripts and module organization, eliminates duplicate FFI implementations, and updates Cargo.toml and .gitignore |
| DashSharedCore.podspec | Removes the debug argument in the build command |
Comments suppressed due to low confidence (3)
dash-spv-platform/src/lib.rs:377
- The updated tuple structure now includes an additional field for TokenPaymentInfo. Ensure that downstream consumers correctly handle this new optional parameter across all document transition methods.
let documents_iter = IndexMap::<DocumentTransitionActionType, Vec<(Document, DocumentTypeRef, Bytes32, Option<TokenPaymentInfo>)>>::from_iter([(action_type, vec![(document, doc_type_ref, Bytes32(entropy), None)])]);
dash-spv-apple-bindings/build.sh:80
- [nitpick] Verify that enabling fermentize for only the first build target is intentional; if multiple targets should include fermentize, adjust the logic to apply the extra feature as needed.
fermentize=1
DashSharedCore.podspec:17
- [nitpick] Removing the 'debug' argument from the build script invocation should be validated to ensure it does not affect local debugging workflows; update documentation if the build process has changed.
./build.sh
| } | ||
|
|
||
| fn get_token_configuration(&self, token_id: &Identifier) -> Result<Option<TokenConfiguration>, ContextProviderError> { | ||
| Err(ContextProviderError::TokenConfigurationFailure(format!("Not implemented {token_id}"))) |
There was a problem hiding this comment.
The error message in get_token_configuration is a placeholder. Consider using the unimplemented!() macro or providing additional context to make future implementation clearer.
Suggested change
| Err(ContextProviderError::TokenConfigurationFailure(format!("Not implemented {token_id}"))) | |
| unimplemented!("get_token_configuration is not implemented for token_id: {token_id}"); |
Comment on lines
+265
to
+272
| let mut d = String::new(); | ||
| d.push_str(format!("\ntip: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)).as_str()); | ||
| d.push_str(format!("\n h: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)).as_str()); | ||
| d.push_str(format!("\n h-c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_c.new_quorums)).as_str()); | ||
| d.push_str(format!("\n h-2c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_2c.new_quorums)).as_str()); | ||
| d.push_str(format!("\n h-3c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_3c.new_quorums)).as_str()); | ||
| d.push_str(format!("\n h-4c{}", qr_info.quorum_snapshot_and_mn_list_diff_at_h_minus_4c.as_ref().map(|(q,qq)| quorum_list_desc(&qq.new_quorums)).unwrap_or_default()).as_str()); | ||
| d.push_str(format!("\n lq/i{}", quorum_list_desc(&qr_info.last_commitment_per_index)).as_str()); |
There was a problem hiding this comment.
[nitpick] Consider refactoring the quorum info logging to use a join on a collection of strings or a multiline format string for improved readability and maintainability.
Suggested change
| let mut d = String::new(); | |
| d.push_str(format!("\ntip: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)).as_str()); | |
| d.push_str(format!("\n h: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)).as_str()); | |
| d.push_str(format!("\n h-c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_c.new_quorums)).as_str()); | |
| d.push_str(format!("\n h-2c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_2c.new_quorums)).as_str()); | |
| d.push_str(format!("\n h-3c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_3c.new_quorums)).as_str()); | |
| d.push_str(format!("\n h-4c{}", qr_info.quorum_snapshot_and_mn_list_diff_at_h_minus_4c.as_ref().map(|(q,qq)| quorum_list_desc(&qq.new_quorums)).unwrap_or_default()).as_str()); | |
| d.push_str(format!("\n lq/i{}", quorum_list_desc(&qr_info.last_commitment_per_index)).as_str()); | |
| let d = vec![ | |
| format!("\ntip: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)), | |
| format!("\n h: {}", quorum_list_desc(&qr_info.mn_list_diff_h.new_quorums)), | |
| format!("\n h-c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_c.new_quorums)), | |
| format!("\n h-2c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_2c.new_quorums)), | |
| format!("\n h-3c{}", quorum_list_desc(&qr_info.mn_list_diff_at_h_minus_3c.new_quorums)), | |
| format!("\n h-4c{}", qr_info.quorum_snapshot_and_mn_list_diff_at_h_minus_4c.as_ref().map(|(q,qq)| quorum_list_desc(&qq.new_quorums)).unwrap_or_default()), | |
| format!("\n lq/i{}", quorum_list_desc(&qr_info.last_commitment_per_index)), | |
| ].join(""); |
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.
This pull request introduces a variety of changes across multiple files, primarily focusing on enabling a new feature (
fermentize), improving error handling, and refining the functionality of the Dash SPV platform and masternode processor. Below is a categorized summary of the most important changes:Feature Enablement:
fermentizefermentizeinCargo.tomland updatedbuild.rsto conditionally includeFermentfunctionality based on the feature flag. This includes handling configurations, conditional compilation, and logging when the feature is disabled. [1] [2] [3] [4]build.shto dynamically include thefermentizefeature during builds for specific targets.Error Handling and FFI Improvements
hashes_hex_Error_FFIenum and its associated FFI conversion implementations for better error handling indashcore.rs.Masternode Processor Enhancements
quorum_list_descfor formatting quorum information, improving code readability and reuse.Dash SPV Platform Updates
document_batch_signed_transitionand related methods to include an optionalTokenPaymentInfoparameter, enhancing document transition capabilities. [1] [2] [3]document_batchmethod, simplifying thePlatformSDKinterface.Miscellaneous Changes
.gitignoreto include thetmpdirectory, preventing temporary files from being tracked.build.shscript by removing thedebugargument from theprepare_commandin theDashSharedCore.podspec.These changes collectively enhance the modularity, error handling, and functionality of the project while preparing it for the new
fermentizefeature.