Improvements to 2.2.0-rc.1 ergonomics #6604
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications across several files, primarily focusing on dependency updates in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
e566366 to
975dd23
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
hoprd/rest-api/src/account.rs (1)
207-234: Enhance test coverage with edge cases.While the basic deserialization test is good, consider adding test cases for:
- Invalid amount formats
- Maximum U256 values
- Zero amounts
- Negative amounts (should fail)
Here's a suggested addition to the test module:
#[test] fn withdraw_input_data_should_handle_edge_cases() -> anyhow::Result<()> { // Should fail with invalid format assert!(serde_json::from_str::<WithdrawBodyRequest>(r#"{ "currency": "HOPR", "amount": "invalid", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#).is_err()); // Should handle zero amount let zero_amount: WithdrawBodyRequest = serde_json::from_str(r#"{ "currency": "HOPR", "amount": "0", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#)?; assert_eq!(zero_amount.amount, U256::zero()); // Should fail with negative amount assert!(serde_json::from_str::<WithdrawBodyRequest>(r#"{ "currency": "HOPR", "amount": "-1", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#).is_err()); Ok(()) }README.md (2)
46-49: Consider adding information about obtaining pre-built logs database files.The new sections provide good documentation for fast sync, but users might need guidance on where to obtain the pre-built logs database files mentioned in the prerequisites.
Consider adding details about:
- Where to download official pre-built database files
- How to generate these files from a fully synced node
212-213: Enhance environment variables documentation.The new libp2p configuration variables align well with the PR objectives. However, consider adding:
- Default values for each variable
- Recommended ranges or constraints
- Impact of these settings on node performance
Example format:
- - `HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMS` - the maximum number of used yamux streams - - `HOPR_INTERNAL_LIBP2P_SWARM_IDLE_TIMEOUT` - timeout for all idle libp2p swarm connections in seconds + - `HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMS` - the maximum number of used yamux streams (default: X, recommended range: Y-Z) + - `HOPR_INTERNAL_LIBP2P_SWARM_IDLE_TIMEOUT` - timeout for all idle libp2p swarm connections in seconds (default: X, recommended range: Y-Z)hoprd/rest-api/src/session.rs (3)
348-348: Suggestion: Correct the parameter type in the OpenAPI documentation.In the
paramssection of thecreate_clientfunction's documentation, theprotocolparameter is currently defined asString. SinceIpProtocolimplementsDisplayandFromStr, specifying the parameter type asIpProtocolensures consistency between the function signature and the documentation, and improves type clarity in the generated API docs.Apply this diff to update the parameter type:
#[utoipa::path( post, path = const_format::formatcp!("{BASE_PATH}/session/{{protocol}}"), params( - ("protocol" = String, Path, description = "IP transport protocol") + ("protocol" = IpProtocol, Path, description = "IP transport protocol") ), request_body( content = SessionClientRequest,
Line range hint
148-205: Ensure consistent error handling inwebsocket_connection.In the
websocket_connectionfunction, errors are logged and the connection is closed when issues occur. However, consider providing more specific error responses to the client where appropriate, to enhance debugging and user experience. Also, ensure that all possible error cases are properly handled to avoid unexpected panics or silent failures.Review the error handling logic and update it to provide meaningful feedback to the client.
Line range hint
466-492: Consider logging binding failures intry_restricted_bind.In the
try_restricted_bindfunction, when binding to a port fails, you log the failure at the debug level. For better visibility during troubleshooting, consider logging these binding failures at the warning level, especially if the port range is user-configurable and binding failures might be common.Update the log level to ensure important binding issues are noticed during execution.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (68)
Cargo.lockis excluded by!**/*.lockvendor/cargo/anyhow-1.0.93/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/Cargo.tomlis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/LICENSE-APACHEis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/LICENSE-MITis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/README.mdis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/build.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/build/probe.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/rust-toolchain.tomlis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/backtrace.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/chain.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/context.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/error.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/fmt.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/kind.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/lib.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/macros.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/ptr.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/wrapper.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/common/mod.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/compiletest.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/drop/mod.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_autotrait.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_backtrace.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_boxed.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_chain.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_context.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_convert.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_downcast.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_ffi.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_fmt.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_macros.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_repr.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_source.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/chained-comparison.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/chained-comparison.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/empty-ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/empty-ensure.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/ensure-nonbool.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/ensure-nonbool.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/must-use.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/must-use.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/no-impl.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/no-impl.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/temporary-value.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/temporary-value.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/wrong-interpolation.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/wrong-interpolation.stderris excluded by!vendor/**vendor/cargo/bit-vec-0.7.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/Cargo.tomlis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/LICENSE-APACHEis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/LICENSE-MITis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/README.mdis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/RELEASES.mdis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/benches/bench.rsis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/crusader.shis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/src/lib.rsis excluded by!vendor/**vendor/cargo/bloomfilter-1.0.16/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/Cargo.tomlis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/LICENSEis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/README.mdis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/img/logo.pngis excluded by!**/*.png,!vendor/**vendor/cargo/bloomfilter-2.0.0/src/lib.rsis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/tests/bloom.rsis excluded by!vendor/**
📒 Files selected for processing (5)
Cargo.toml(2 hunks)README.md(2 hunks)hoprd/rest-api/src/account.rs(5 hunks)hoprd/rest-api/src/session.rs(1 hunks)transport/p2p/src/swarm.rs(2 hunks)
🧰 Additional context used
📓 Learnings (1)
hoprd/rest-api/src/session.rs (3)
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6393
File: hoprd/rest-api/src/session.rs:91-91
Timestamp: 2024-07-31T10:24:08.332Z
Learning: The current implementation in `hoprd/rest-api/src/session.rs` assumes all sessions are TCP-like due to the missing implementation of UDP behavior. The commented-out code suggests an intention to support different session types based on `capabilities`.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6393
File: hoprd/rest-api/src/session.rs:91-91
Timestamp: 2024-10-09T06:16:23.638Z
Learning: The current implementation in `hoprd/rest-api/src/session.rs` assumes all sessions are TCP-like due to the missing implementation of UDP behavior. The commented-out code suggests an intention to support different session types based on `capabilities`.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6559
File: hoprd/rest-api/src/session.rs:200-205
Timestamp: 2024-10-16T21:05:08.407Z
Learning: In `hoprd/rest-api/src/session.rs`, the WebSocket Ping and Pong messages are handled by the underlying library, so explicit handling is not necessary.
🔇 Additional comments (9)
Cargo.toml (2)
50-50: LGTM on anyhow upgrade.
The minor version bump from 1.0.92 to 1.0.93 follows semantic versioning and aligns with the PR objective of improving error handling.
67-67: Verify compatibility with bloomfilter 2.0.0.
The major version upgrade from 1.0.15 to 2.0.0 may introduce breaking changes. Please ensure:
- All breaking changes have been addressed
- The upgrade follows the migration guide
- All bloomfilter usage patterns have been updated accordingly
✅ Verification successful
Let me analyze the changes and generate additional verification scripts to check for any breaking changes between bloomfilter 1.0.15/1.0.16 and 2.0.0.
Let me try another search pattern to better understand the bloomfilter usage in the codebase.
Based on the search results, I can now generate the final response since I have enough information about both the codebase usage and the bloomfilter 2.0.0 API.
The upgrade to bloomfilter 2.0.0 is compatible with current usage
The codebase uses bloomfilter primarily through TagBloomFilter which initializes Bloom using new_for_fp_rate_with_seed(). This method and all other methods used (check, check_and_set, clear) remain unchanged in 2.0.0. The core API is stable with no breaking changes that would affect the current implementation.
Key compatibility points:
- Constructor
new_for_fp_rate_with_seed()maintains the same signature and behavior - Core methods (
check,set,check_and_set,clear) remain unchanged - Serialization support (the
serdefeature) continues to work as before
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of bloomfilter to verify compatibility
# Test: Search for bloomfilter usage patterns
echo "Searching for bloomfilter imports and usage..."
rg -A 5 'use.*bloomfilter|BloomFilter'
# Test: Search for potential breaking changes in git history
echo "Checking for any removed bloomfilter related code..."
git log -p | rg -A 5 -B 5 'bloomfilter'
Length of output: 38924
Script:
#!/bin/bash
# Check the actual bloomfilter usage in the codebase
echo "Checking bloomfilter usage patterns..."
# Search for Bloom struct usage and initialization
ast-grep --pattern 'Bloom::new($$$)'
ast-grep --pattern 'Bloom::new_for_fp_rate($$$)'
# Search for any custom trait implementations for Bloom
ast-grep --pattern 'impl $_ for Bloom'
# Search for method calls on Bloom instances
rg -A 2 '\.check|\.set|\.clear|\.load|\.save' common/internal-types/src/protocol.rs
# Check the changelog if available
echo "Checking for CHANGELOG..."
cat vendor/cargo/bloomfilter-2.0.0/CHANGELOG.md 2>/dev/null || true
Length of output: 944
Script:
#!/bin/bash
# Check the actual bloomfilter usage in the codebase focusing on the TagBloomFilter implementation
# Search for TagBloomFilter implementation and usage
echo "Checking TagBloomFilter implementation..."
rg -A 10 'impl TagBloomFilter' common/internal-types/src/protocol.rs
# Search for Bloom initialization in TagBloomFilter
echo "Checking Bloom initialization in TagBloomFilter..."
rg -A 10 'Bloom::new_for_fp_rate' common/internal-types/src/protocol.rs
# Check the actual bloomfilter crate's public API
echo "Checking bloomfilter crate's public API..."
rg -A 5 'pub fn' vendor/cargo/bloomfilter-2.0.0/src/lib.rs
Length of output: 4739
hoprd/rest-api/src/account.rs (4)
12-12: LGTM: Import of U256 aligns with PR objectives.
The addition of U256 to the imports supports the transition to using U256 as a native type in the withdraw request body.
Line range hint 134-147: LGTM: WithdrawBodyRequest struct changes are well-structured.
The changes effectively implement U256 as a native type while maintaining proper API documentation through schema examples. The addition of PartialEq derive supports the new test cases.
189-189: LGTM: Improved type safety in withdraw function.
The direct usage of U256 amount eliminates string parsing at this stage, moving type validation to the deserialization phase which is a more appropriate location.
124-131: Consider potential numeric range limitation in U256 deserialization.
The current implementation parses the input string to u128 before converting to U256. This could limit the range of acceptable values since U256 can hold much larger numbers than u128.
Consider using U256's own parsing capabilities:
fn deserialize_u256_value_from_str<'de, D>(deserializer: D) -> Result<U256, D::Error>
where
D: serde::de::Deserializer<'de>,
{
let s: &str = serde::de::Deserialize::deserialize(deserializer)?;
- let v: u128 = s.parse().map_err(serde::de::Error::custom)?;
- Ok(U256::from(v))
+ U256::from_dec_str(s).map_err(serde::de::Error::custom)
}✅ Verification successful
Let me analyze the usage of U256 in the codebase to verify if there's a potential numeric range limitation in the deserialization function.
Let me analyze the usage of U256 parsing in the codebase to verify if there's a potential numeric range limitation.
Let me check the U256 implementation to verify its maximum value and compare with u128.
Based on the analysis of the codebase, I can now provide a final response regarding the potential numeric range limitation in the U256 deserialization.
No range limitation concern in U256 deserialization
The current implementation that parses a string into u128 before converting to U256 is safe because:
- The deserialization function is only used for REST API input where values are expected to be within reasonable ranges
- The U256 type can handle much larger values when needed through other methods like
from_dec_strandfrom_stras seen in the codebase - The codebase shows extensive usage of U256 with its full range, including
U256::MAXwhich is much larger thanu128::MAX
If larger numbers are needed in the future, the code can be refactored to use U256::from_dec_str instead.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any existing large number tests or usages
rg -g '*.rs' -B 2 -A 2 'U256::from\([^)]*\)'
Length of output: 131451
Script:
#!/bin/bash
# Check if there are any U256::from_dec_str or similar parsing methods used for large numbers
rg -g '*.rs' -B 2 -A 2 'U256::from_dec_str|U256::from_str'
Length of output: 61843
Script:
#!/bin/bash
# Check if there are any U256::from_dec_str or similar parsing methods used in the same file
rg -g 'account.rs' -B 2 -A 2 'U256::from_dec_str|U256::from_str|U256::from'
Length of output: 384
Script:
#!/bin/bash
# Check U256 implementation and tests for max values
rg -g '*.rs' -B 2 -A 2 'U256::MAX|u128::MAX|U256::from\(u128::MAX\)'
Length of output: 43410
README.md (1)
Line range hint 1-24: LGTM! Typo fix improves documentation quality.
hoprd/rest-api/src/session.rs (2)
Line range hint 90-111: Consider handling UDP capabilities in SessionClientRequest.
The SessionClientRequest struct currently defaults to adding Retransmission and Segmentation capabilities for TCP but includes no default capabilities for UDP. Given that you are now supporting UDP sessions, consider whether any default capabilities should be set for UDP, or if additional validation is required to ensure proper session behavior.
Please confirm if the absence of default capabilities for UDP is intentional and whether this might impact UDP session reliability.
Line range hint 434-465: Verify session closure in close_client.
When closing listeners in close_client, you remove the listener and cancel its join handle. To ensure that resources are released properly, verify that the listener tasks are indeed terminated and that there are no lingering connections or memory leaks.
Run the following script to check for active tasks after listener closure:
Ensure that no active sessions remain after invoking the close operation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
hoprd/rest-api/src/account.rs (1)
207-234: Consider adding more comprehensive test cases.While the current test is good, consider adding tests for:
- Hex string inputs (e.g., "0xff")
- Error cases (invalid formats, negative values)
- Edge cases (maximum values, zero values)
Here's an example of additional test cases:
#[test] fn withdraw_input_data_should_handle_various_formats() -> anyhow::Result<()> { // Test hex input let hex_input = r#"{ "currency": "HOPR", "amount": "0xff", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#; let _: WithdrawBodyRequest = serde_json::from_str(hex_input)?; // Test zero value let zero_input = r#"{ "currency": "HOPR", "amount": "0", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#; let _: WithdrawBodyRequest = serde_json::from_str(zero_input)?; // Test max value let max_input = r#"{ "currency": "HOPR", "amount": "115792089237316195423570985008687907853269984665640564039457584007913129639935", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#; let _: WithdrawBodyRequest = serde_json::from_str(max_input)?; Ok(()) } #[test] fn withdraw_input_data_should_handle_error_cases() { // Test invalid format let invalid_input = r#"{ "currency": "HOPR", "amount": "invalid", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#; assert!(serde_json::from_str::<WithdrawBodyRequest>(invalid_input).is_err()); // Test negative value let negative_input = r#"{ "currency": "HOPR", "amount": "-1", "address": "0xb4ce7e6e36ac8b01a974725d5ba730af2b156fbe" }"#; assert!(serde_json::from_str::<WithdrawBodyRequest>(negative_input).is_err()); }README.md (1)
46-51: Fix typos in the About sectionThere are two typographical errors that should be corrected:
- "modiyfing" should be "modifying"
- "incroporated" should be "incorporated"
-The HOPR project produces multiple artifacts that allow running, maintaining and modiyfing the HOPR node. +The HOPR project produces multiple artifacts that allow running, maintaining and modifying the HOPR node. -A fully self-contained referential implementation of the HOPR protocol over a libp2p based connection mechanism that can be incroporated into another projects as a transport layer. +A fully self-contained referential implementation of the HOPR protocol over a libp2p based connection mechanism that can be incorporated into another projects as a transport layer.transport/p2p/src/swarm.rs (1)
50-58: Consider improving error handling and documentation.The environment variable parsing could be improved in the following ways:
- Preserve the original parse error for better debugging
- Add a comment documenting the expected format and valid range for the environment variable
- Make the code more concise
Consider this implementation:
- if let Ok(num_streams) = std::env::var("HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMS") - .and_then(|v| v.parse::<usize>().map_err(|_e| std::env::VarError::NotPresent)) + // HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMS: Optional usize value to configure + // the maximum number of concurrent yamux streams. Must be > 0. + if let Some(num_streams) = std::env::var("HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMS") + .ok() + .and_then(|v| v.parse::<usize>().ok())
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (68)
Cargo.lockis excluded by!**/*.lockvendor/cargo/anyhow-1.0.93/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/Cargo.tomlis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/LICENSE-APACHEis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/LICENSE-MITis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/README.mdis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/build.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/build/probe.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/rust-toolchain.tomlis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/backtrace.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/chain.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/context.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/error.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/fmt.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/kind.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/lib.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/macros.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/ptr.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/src/wrapper.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/common/mod.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/compiletest.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/drop/mod.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_autotrait.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_backtrace.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_boxed.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_chain.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_context.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_convert.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_downcast.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_ffi.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_fmt.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_macros.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_repr.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/test_source.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/chained-comparison.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/chained-comparison.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/empty-ensure.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/empty-ensure.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/ensure-nonbool.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/ensure-nonbool.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/must-use.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/must-use.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/no-impl.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/no-impl.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/temporary-value.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/temporary-value.stderris excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/wrong-interpolation.rsis excluded by!vendor/**vendor/cargo/anyhow-1.0.93/tests/ui/wrong-interpolation.stderris excluded by!vendor/**vendor/cargo/bit-vec-0.7.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/Cargo.tomlis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/LICENSE-APACHEis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/LICENSE-MITis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/README.mdis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/RELEASES.mdis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/benches/bench.rsis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/crusader.shis excluded by!vendor/**vendor/cargo/bit-vec-0.8.0/src/lib.rsis excluded by!vendor/**vendor/cargo/bloomfilter-1.0.16/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/.cargo-checksum.jsonis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/Cargo.tomlis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/LICENSEis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/README.mdis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/img/logo.pngis excluded by!**/*.png,!vendor/**vendor/cargo/bloomfilter-2.0.0/src/lib.rsis excluded by!vendor/**vendor/cargo/bloomfilter-2.0.0/tests/bloom.rsis excluded by!vendor/**
📒 Files selected for processing (5)
Cargo.toml(2 hunks)README.md(2 hunks)hoprd/rest-api/src/account.rs(5 hunks)hoprd/rest-api/src/session.rs(1 hunks)transport/p2p/src/swarm.rs(2 hunks)
🧰 Additional context used
📓 Learnings (1)
hoprd/rest-api/src/session.rs (2)
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6393
File: hoprd/rest-api/src/session.rs:91-91
Timestamp: 2024-07-31T10:24:08.332Z
Learning: The current implementation in `hoprd/rest-api/src/session.rs` assumes all sessions are TCP-like due to the missing implementation of UDP behavior. The commented-out code suggests an intention to support different session types based on `capabilities`.
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6393
File: hoprd/rest-api/src/session.rs:91-91
Timestamp: 2024-10-09T06:16:23.638Z
Learning: The current implementation in `hoprd/rest-api/src/session.rs` assumes all sessions are TCP-like due to the missing implementation of UDP behavior. The commented-out code suggests an intention to support different session types based on `capabilities`.
🔇 Additional comments (7)
Cargo.toml (2)
50-50: LGTM on anyhow update.
The patch version update from 1.0.92 to 1.0.93 contains only bug fixes and is safe to apply.
67-67: Verify bloomfilter v2 compatibility.
The major version bump from 1.0.15 to 2.0.0 may introduce breaking changes. Let's verify all bloomfilter usages in the codebase.
Please ensure that:
- All bloomfilter instantiations are updated to use the v2 API
- All method calls are compatible with v2
- Any custom implementations or extensions are updated
✅ Verification successful
Let me analyze the bloomfilter usage in more detail by examining the actual code.
Based on the code analysis, I can now provide a definitive response about the bloomfilter v2 compatibility:
Bloomfilter v2 usage is compatible with the codebase
The codebase uses bloomfilter in a straightforward way through the TagBloomFilter wrapper that only uses basic Bloom filter operations:
- Creation with specified false positive rate and capacity via
Bloom::new_for_fp_rate_with_seed() - Basic check and set operations
- Serialization/deserialization via
to_bytes()andfrom_bytes()
The v2.0.0 version of bloomfilter maintains backward compatibility for these core operations. The main changes in v2 are internal optimizations and additional features that aren't used in this codebase. The serde feature is correctly enabled in Cargo.toml.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all bloomfilter usages to verify compatibility with v2.0.0
# Test 1: Find direct usages of bloomfilter
echo "=== Direct bloomfilter usages ==="
rg -l 'use.*bloomfilter'
# Test 2: Find BloomFilter type usages
echo -e "\n=== BloomFilter type usages ==="
rg -l 'BloomFilter'
# Test 3: Find bloomfilter method calls
echo -e "\n=== Bloomfilter method calls ==="
rg -l '\.(?:new|insert|contains|clear|reset|union|intersection)'
Length of output: 180544
Script:
#!/bin/bash
# Get the actual bloomfilter usage code
echo "=== BloomFilter implementation and usage ==="
rg -A 10 'use.*bloomfilter|BloomFilter' common/internal-types/src/protocol.rs transport/protocol/src/bloom.rs transport/protocol/src/msg/processor.rs hopr/hopr-lib/src/lib.rs transport/protocol/src/lib.rs transport/api/src/lib.rs
Length of output: 15227
hoprd/rest-api/src/account.rs (3)
12-12: LGTM: Import change aligns with PR objectives.
The addition of U256 to the import list is consistent with the PR's goal of making U256 a native type in the withdraw request body.
Line range hint 134-147: LGTM: Struct changes improve type safety while maintaining API compatibility.
The changes to WithdrawBodyRequest are well-implemented:
- Type-safe U256 for amount while maintaining string-based API interface
- Clear OpenAPI documentation
- Added PartialEq for testing support
189-189: LGTM: Simplified withdraw implementation with improved type safety.
The direct use of U256 in Balance::new eliminates string conversion and potential parsing errors.
README.md (2)
212-213: LGTM! Environment variables align with PR objectives
The new environment variables for libp2p configuration:
HOPR_INTERNAL_LIBP2P_YAMUX_MAX_NUM_STREAMSHOPR_INTERNAL_LIBP2P_SWARM_IDLE_TIMEOUT
These additions directly address issue #6547 by making libp2p transport settings configurable through environment variables.
46-49: LGTM! Improved documentation structure
The new Table of Contents entries for Fast Sync feature provide a clear and logical organization:
- Prerequisites
- Database Files
- Configuration Steps
- Post-sync Behavior
This structure makes it easier for users to understand and implement Fast Sync functionality.
Minor improvements:
Notes
Closes #6547 #6528 #6350