Fix clippy warnings #7180
Fix clippy warnings #7180
Conversation
📝 WalkthroughWalkthroughThis set of changes is focused on test and style improvements across multiple modules. The updates primarily remove unnecessary clones, dereferences, and conversions, standardize type usage, enhance error handling in tests, and simplify assertions and iteration patterns. No core logic, control flow, or exported/public API signatures are altered. Changes
Sequence Diagram(s)No sequence diagrams are generated as the changes are limited to test/style improvements and do not affect control flow or introduce new features. Possibly related PRs
Suggested labels
✨ 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
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a series of clippy warnings across the codebase by replacing unnecessary cloning and using more idiomatic constructs. Key changes include replacing .clone() calls with dereference operators for public keys, substituting std::iter::repeat with std::iter::repeat_n, and updating U256 conversion methods.
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crypto/sphinx/src/surb.rs | Replace clone on public key with dereference to improve efficiency. |
| crypto/sphinx/src/derivation.rs | Adjust public key and slice handling to avoid unnecessary clones. |
| crypto/packet/src/types.rs & crypto/packet/src/packet.rs | Update public key extraction to remove cloning in tests. |
| common/network-types/src/session/state.rs | Use repeat_n for clarity and idiomatic iteration. |
| chain/rpc/src/rpc.rs | Update U256 conversion by using from_big_endian with byte vector slices. |
| chain/actions/src/node.rs | Replace cloning with dereference for public key extraction in tests. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
hoprd/rest-api/src/node.rs (1)
314-317: Consider preserving original error information for better debugging.The new function implementation correctly maps prometheus errors to
ApiErrorStatus, but using|_|discards the original error details which could be valuable for debugging and monitoring.Consider preserving the original error information:
-fn collect_hopr_metrics() -> Result<String, ApiErrorStatus> { - hopr_metrics::metrics::gather_all_metrics() - .map_err(|_| ApiErrorStatus::UnknownFailure("Failed to gather metrics".into())) -} +fn collect_hopr_metrics() -> Result<String, ApiErrorStatus> { + hopr_metrics::metrics::gather_all_metrics() + .map_err(|e| ApiErrorStatus::UnknownFailure(format!("Failed to gather metrics: {}", e))) +}This maintains the same error type while providing more context for troubleshooting.
hopli/src/methods.rs (4)
1499-1504: Remove unnecessary.into()onAddress.Both
grantRoleandhasRoleacceptAddressdirectly, so the.into()conversion is redundant.
1509-1513: Drop redundant.into()onhasRole.You can call
hasRole(encoded_minter_role, contract_deployer.public().to_address())without converting.
1555-1563: Remove.into()on addresses forgrantRole.As above, passing the
Addressdirectly is sufficient.
1563-1571: Drop redundant.into()onhasRole.Call
hasRole(encoded_minter_role, contract_deployer.public().to_address())directly for cleaner code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
chain/actions/src/channels.rs(4 hunks)chain/actions/src/node.rs(3 hunks)chain/actions/src/redeem.rs(3 hunks)chain/indexer/src/block.rs(2 hunks)chain/indexer/src/handlers.rs(61 hunks)chain/rpc/src/client.rs(2 hunks)chain/rpc/src/indexer.rs(1 hunks)chain/rpc/src/rpc.rs(5 hunks)common/internal-types/src/tickets.rs(2 hunks)common/network-types/benches/session.rs(1 hunks)common/network-types/src/session/protocol.rs(3 hunks)common/network-types/src/session/state.rs(7 hunks)common/network-types/src/session/utils.rs(1 hunks)crypto/packet/benches/packet_benches.rs(9 hunks)crypto/packet/benches/por_benches.rs(1 hunks)crypto/packet/src/lib.rs(1 hunks)crypto/packet/src/packet.rs(2 hunks)crypto/packet/src/types.rs(1 hunks)crypto/sphinx/src/derivation.rs(2 hunks)crypto/sphinx/src/surb.rs(1 hunks)crypto/types/src/vrf.rs(2 hunks)db/sql/src/channels.rs(1 hunks)db/sql/src/peers.rs(3 hunks)db/sql/src/tickets.rs(1 hunks)hopli/src/environment_config.rs(4 hunks)hopli/src/key_pair.rs(22 hunks)hopli/src/methods.rs(19 hunks)hopr/hopr-lib/tests/chain_integration_tests.rs(1 hunks)hopr/hopr-lib/tests/common/mod.rs(2 hunks)hoprd/keypair/src/key_pair.rs(2 hunks)hoprd/rest-api/src/node.rs(2 hunks)logic/strategy/src/aggregating.rs(2 hunks)logic/strategy/src/auto_funding.rs(1 hunks)transport/mixer/src/channel.rs(6 hunks)transport/ticket-aggregation/src/lib.rs(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
common/internal-types/src/tickets.rs (1)
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6511
File: db/sql/src/tickets.rs:1122-1133
Timestamp: 2024-10-09T06:16:18.170Z
Learning: When comparing winning probabilities in floating-point calculations, avoid oversimplifying conditions and ensure that equality cases are correctly handled when `LOWEST_POSSIBLE_WINNING_PROB` is close to `f64::EPSILON`.
crypto/packet/benches/packet_benches.rs (1)
Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6619
File: crypto/packet/benches/packet_crypto.rs:74-120
Timestamp: 2024-11-11T18:29:26.659Z
Learning: In the `crypto/packet/benches/packet_crypto.rs` benchmarks, generalizing initialization code to reduce duplication is preferred, but alternative approaches are favored over the previously suggested fixture pattern.
db/sql/src/tickets.rs (1)
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6558
File: db/sql/src/tickets.rs:331-343
Timestamp: 2024-10-16T19:21:44.890Z
Learning: In the `mark_tickets_as` function in `db/sql/src/tickets.rs`, `marked_value.amount()` is always less than or equal to `unredeemed_value`, ensuring that subtraction does not underflow.
🧬 Code Graph Analysis (5)
crypto/sphinx/src/derivation.rs (1)
crypto/types/src/vrf.rs (1)
derive_vrf_parameters(196-239)
chain/actions/src/channels.rs (1)
common/internal-types/src/channels.rs (1)
generate_channel_id(169-171)
hoprd/rest-api/src/node.rs (2)
sdk/python/api/hopr.py (1)
metrics(345-348)misc/metrics/src/metrics.rs (1)
gather_all_metrics(7-12)
common/network-types/src/session/state.rs (3)
common/network-types/src/session/protocol.rs (1)
iter(672-672)crypto/packet/src/types.rs (1)
data(121-121)chain/actions/src/action_state.rs (1)
test(82-84)
common/network-types/src/session/protocol.rs (2)
common/network-types/src/session/state.rs (1)
std(1141-1141)db/sql/src/db.rs (1)
std(175-179)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: hopli / docker
- GitHub Check: hoprd / docker
- GitHub Check: tests-smoke-hopli
- GitHub Check: tests-unit-nightly
- GitHub Check: tests-unit
- GitHub Check: Linter
- GitHub Check: zizmor
- GitHub Check: Cargo Audit
🔇 Additional comments (83)
logic/strategy/src/auto_funding.rs (2)
196-196: LGTM! Removed unnecessary clone operation.Direct assignment is more efficient since
Balancelikely implementsCopy. This change fixes the clippy warning about unnecessary cloning while maintaining the same functionality.
200-200: LGTM! Simplified comparison logic.The direct value comparison is cleaner and more readable than the previous reference-based comparison. This change addresses clippy warnings while preserving the same comparison semantics.
hoprd/rest-api/src/node.rs (1)
343-343: LGTM! Improved error handling efficiency.The change removes unnecessary error conversion and returns the
ApiErrorStatusdirectly, which is more efficient and aligns with clippy best practices for avoiding redundant conversions.hopli/src/environment_config.rs (4)
269-273: LGTM! Test assertion simplified effectively.The replacement of explicit
matchstatements with directassert!calls using the?operator is a clean improvement. This change makes the test more concise while maintaining the same error propagation behavior.
309-313: LGTM! Consistent test assertion pattern applied.Similar to the previous test, this change appropriately uses
assert!(!...)to verify the function returnsfalse, while leveraging the?operator for error propagation. The logic remains identical but more readable.
328-328: LGTM! Unnecessary conversion removed.Removing
.into()fromanvil.endpoint()is correct since the method likely already returns a compatible type for theStringfield. This addresses clippy warnings about unnecessary conversions.
349-349: LGTM! Consistent removal of unnecessary conversion.Same improvement as line 328 - the
.into()conversion is redundant and its removal improves code clarity while addressing clippy warnings.common/network-types/benches/session.rs (1)
35-35: LGTM! Unnecessary reference operator removed.The change correctly removes the redundant reference operator. Since
datais already&[u8], passing&datawould create&&[u8], butwrite_all()expects&[u8]. This fix addresses the clippy warning about unnecessary referencing.common/network-types/src/session/utils.rs (1)
243-243: LGTM! Explicitly discarding unused result improves clarity.The change makes the intent explicit that the write result is intentionally ignored while still maintaining error handling with
.unwrap(). This addresses the clippy warning about unused results and improves code readability.chain/rpc/src/indexer.rs (1)
421-426: LGTM! More efficient iterator usage.The change from
.last()to.next_back()is a good optimization. While.last()consumes the entire iterator to reach the last element,.next_back()efficiently gets the last element from a double-ended iterator without consuming the whole sequence. This addresses the clippy performance warning.db/sql/src/peers.rs (4)
452-452: LGTM! Fixed malformed floating-point literal.The change from
0.4_64to0.4_f64corrects the invalid numeric literal syntax. The proper way to specify an f64 type annotation is with_f64, not_64.
483-483: LGTM! Fixed malformed floating-point literal.The change from
0.4_64to0.4_f64corrects the invalid numeric literal syntax. The proper way to specify an f64 type annotation is with_f64, not_64.
547-547: LGTM! Fixed malformed floating-point literal.The change from
0.4_64to0.4_f64corrects the invalid numeric literal syntax. The proper way to specify an f64 type annotation is with_f64, not_64.
621-621: LGTM! Fixed malformed floating-point literal.The change from
0.4_64to0.4_f64corrects the invalid numeric literal syntax. The proper way to specify an f64 type annotation is with_f64, not_64.transport/mixer/src/channel.rs (6)
333-334: LGTM: Improved test assertion pattern.The separation of assertion from the
Ok(())return is cleaner and more idiomatic than wrapping assertions insideOk(). This improves test readability and follows Rust best practices.
357-358: LGTM: Consistent test assertion improvement.Same beneficial pattern applied consistently across test functions.
408-409: LGTM: Clean test assertion pattern.The assertion separation continues to improve test code clarity.
433-434: LGTM: Consistent assertion improvements.Well-applied clippy fix for better test structure.
459-460: LGTM: Improved assertion pattern.Continues the consistent improvement of test assertion handling.
487-488: LGTM: Final assertion improvement.The complete set of test assertion improvements maintains consistency across all test functions.
db/sql/src/channels.rs (1)
192-192: LGTM: Explicit type conversion improves clarity.Replacing
.into()withU256::from_big_endian(channel_epoch.as_slice())makes the conversion explicit and self-documenting. This improves code readability by clearly indicating that we're converting big-endian bytes to a U256.hopli/src/key_pair.rs (5)
492-492: LGTM: Added context support for better error handling.Adding
anyhow::Contextenables context-aware error messages throughout the test module.
523-523: LGTM: Improved error handling with context.Replacing
.unwrap()calls with.context("should produce a valid tmp path string")?provides much better error messages and follows Rust error handling best practices. This makes test failures more debuggable.Also applies to: 534-534, 555-555, 589-589, 613-613, 629-629, 640-640, 654-654, 667-667, 681-681, 696-696, 829-829, 909-909, 928-928, 947-947, 972-972
507-507: LGTM: Removed unnecessary reference.The
hex::decodefunction takes a value, not a reference, so removing the&is correct and improves clarity.
525-525: LGTM: Simplified function calls by removing unnecessary references.These changes remove unnecessary references and dereferencing, making the code cleaner and more efficient. Good clippy fixes for better code style.
Also applies to: 542-542, 565-565, 575-575, 595-595, 618-618, 644-644, 658-658, 671-671, 686-686, 710-710
645-645: LGTM: Simplified file operations.Removing unnecessary references in
fs::writecalls improves code clarity and follows Rust idioms.Also applies to: 844-844
hoprd/keypair/src/key_pair.rs (1)
546-547: LGTM: Removed unnecessary string conversions.Since the
IdentityRetrievalModes::FromFileexpects&strforpasswordandid_pathfields, and string literals are already&str, the.into()calls were redundant. This improves performance and code clarity.Also applies to: 574-575
crypto/packet/src/lib.rs (1)
125-127: LGTM! Excellent improvement to compile-time verification.Converting the runtime assertion to a compile-time assertion using
const _: () = { ... };is a great improvement. This ensures the constraintHoprPacket::MAX_SURBS_IN_PACKET >= 2is verified during compilation rather than at runtime, providing stronger guarantees and catching violations earlier in the development cycle.chain/rpc/src/client.rs (2)
1465-1465: Good simplification by removing unnecessary conversion.Removing the
.into()call when passing the transaction toprovider.send_transaction()is a good improvement. The transaction builder can be passed directly, making the code cleaner and avoiding unnecessary conversions.
1513-1513: Consistent improvement by removing unnecessary conversion.Same good practice as the previous change - removing the unnecessary
.into()conversion simplifies the code and improves readability.crypto/sphinx/src/surb.rs (1)
226-226: LGTM! Efficient use of Copy trait instead of explicit cloning.The change from
.clone()to dereferencing with*is a good improvement. Since the public key type implementsCopy(as shown in the type bounds), dereferencing directly copies the value more efficiently than callingclone(). This is the idiomatic way to handle Copy types and addresses the clippy warning appropriately.crypto/packet/benches/por_benches.rs (1)
27-27: LGTM! Performance improvement in benchmark by using Copy trait.Excellent change from
.clone()to dereferencing with*for the public key. In a benchmark context, this efficiency improvement is particularly valuable. SinceOffchainPublicKeyimplementsCopy, dereferencing is the more appropriate and performant approach. This change is consistent with the pattern established across the codebase.hopr/hopr-lib/tests/chain_integration_tests.rs (1)
807-808: LGTM! Excellent Clippy warning fix.This change correctly removes the redundant closure by passing the function
cancel_join_handledirectly tomap. The original closure|handle| cancel_join_handle(handle)is functionally equivalent to justcancel_join_handle.chain/indexer/src/block.rs (2)
641-641: LGTM! Removed unnecessary type conversion.The
.into()call after.abi_encode()was redundant since the ABI-encoded data is already in the correct format expected by thedatafield.
1001-1001: LGTM! Using fixed-size array instead of Vec.Changing from
Vec<u64>to[u64; 3]is more efficient since the size is known at compile time. This addresses Clippy warnings about unnecessary heap allocations for fixed-size collections.crypto/packet/src/types.rs (1)
200-200: LGTM! Replaced clone with copy for Copy types.Both changes correctly replace
.clone()with dereferencing*forOffchainPublicKeyvalues. SinceOffchainPublicKeyimplements theCopytrait, using*k.public()is more efficient thank.public().clone()and addresses Clippy'sclone_on_copywarning.Also applies to: 205-205
db/sql/src/tickets.rs (1)
1493-1494: LGTM! Improved iteration pattern addresses Clippy warnings.The change from indexed iteration to direct iteration using
.iter().take()is a good improvement. This pattern is more idiomatic Rust and eliminates potential Clippy warnings about manual indexing when iterating over a known number of elements.chain/actions/src/node.rs (3)
135-135: LGTM! Efficient public key copying.Using dereferencing
*ALICE_OFFCHAIN.public()instead of.clone()is more efficient since public keys implement theCopytrait. This addresses Clippy warnings about unnecessary cloning of Copy types.
147-147: LGTM! Consistent with Copy trait optimization.Same improvement as line 135 - using dereferencing for Copy types is more efficient than cloning.
215-215: LGTM! Simplified reference handling.The change from
&*ALICE_OFFCHAINto&ALICE_OFFCHAINremoves unnecessary dereferencing and re-referencing, making the code cleaner and addressing Clippy warnings.crypto/packet/src/packet.rs (2)
447-447: LGTM: Efficient use of Copy trait instead of cloning.This change replaces
.clone()with dereference (*) for the public key, which is more efficient when the type implementsCopy. This is a good clippy warning fix that improves performance without changing functionality.
587-590: LGTM: Consistent use of Copy semantics for public keys.The changes from
.clone()to*dereference are correct and more efficient for public key types that implementCopy. This pattern is consistently applied across all theprev_hopassignments and aligns with the broader code optimization theme.chain/actions/src/channels.rs (1)
656-656: LGTM: Simplified reference passing to generate_channel_id.The changes correctly pass
&ALICEand&BOBdirectly togenerate_channel_idsince the function expects&Addressparameters and these areAddressvalues. This removes unnecessary dereferencing operations that clippy would flag as redundant.Also applies to: 703-703, 763-763, 810-810
hopr/hopr-lib/tests/common/mod.rs (2)
19-19: LGTM: Appropriate dead code allowance for test utility.The
#[allow(dead_code)]attribute is correctly applied to this test utility function, which may not be used in all test configurations. This is a standard way to suppress clippy warnings for legitimate test infrastructure code.
30-33: LGTM: Simplified return statement improves readability.Removing the intermediate
rpc_clientvariable and returning the builder chain directly makes the code more concise and readable. This is a good clippy warning fix that eliminates unnecessary variable binding.transport/ticket-aggregation/src/lib.rs (2)
598-599: LGTM: Simplified iterator usage removes unnecessary mapping.The changes to use
.into_iter().collect()directly instead of explicit mapping are more idiomatic and efficient. This removes redundant operations that clippy would flag while maintaining the same functionality.Also applies to: 718-719
756-756: LGTM: Enumerated iterator improves readability and safety.Switching from indexed access to
enumerate()is more idiomatic Rust and avoids potential bounds checking issues. The(i, stored_acked_ticket)pattern is clearer thanstored_acked_tickets[i]and aligns with Rust best practices.common/network-types/src/session/protocol.rs (3)
498-500: Excellent improvement: compile-time assertionMoving the size constraint check to compile-time is a great improvement. This ensures the constraint is validated at build time rather than runtime, improving both performance and reliability.
668-668: Good improvement: usingrepeat_nfor better performanceReplacing
.repeat(0).take(10)with.repeat_n(0, 10)is more efficient and idiomatic. Therepeat_nfunction is specifically designed for this use case and avoids the overhead of thetakeadapter.
697-697: Consistent improvement withrepeat_nSame improvement as the previous usage - using
repeat_nis more efficient and idiomatic thanrepeat().take().crypto/sphinx/src/derivation.rs (1)
121-121: Good simplification: removing unnecessary references and dereferencesThese changes remove unnecessary referencing and dereferencing operations in the test code:
messagecan be passed directly toderive_vrf_parameterssince it acceptsAsRef<[u8]>&messageis more direct than&*TEST_MSGpub_key.to_address().as_ref()is cleaner than dereferencing static referencesThe changes maintain identical semantics while improving code clarity and potentially performance.
Also applies to: 124-124, 138-142
logic/strategy/src/aggregating.rs (2)
340-353: Performance improvement: using array literals for static dataReplacing
vec![...]with array literals[...]for the static keypair data is more efficient as arrays are stack-allocated. The final result is the same after mapping and collecting, but the initial allocation is more efficient.
485-485: Good simplification: removing unnecessary explicit mappingRemoving the explicit
.map(TransferableWinningTicket::from)call suggests that the types already match or the conversion is handled implicitly. This simplifies the code by removing an unnecessary transformation step.crypto/types/src/vrf.rs (1)
262-262: Clean simplification: removing redundant reference operationsThese changes remove unnecessary dereference-then-reference operations in the test code:
*TEST_MSGinstead of&*TEST_MSGforderive_vrf_parameterswhich acceptsAsRef<[u8]>&ALICE_ADDRand&TEST_MSGinstead of&*ALICE_ADDRand&*TEST_MSGfor theverifymethodThe changes maintain identical semantics while making the code cleaner and more direct.
Also applies to: 270-270, 293-293, 297-297
hopli/src/methods.rs (10)
1389-1393: Good use ofis_empty()for clarity.Replacing
code.len() == 0withcode.is_empty()improves readability and follows idiomatic Rust.
1467-1470: Test now directly invokes the updated helper.Calling
get_native_and_token_balances(instances.token, vec![kp_address]).await?aligns with the simplified API.
1520-1523: Updated test uses the new multi-balance API.The signature
get_native_and_token_balances(instances.token, addresses.clone())correctly reflects the refactored method.
1538-1542: Simplify single-recipient test setup.Directly constructing a
Vec<Address>improves clarity and matches other test patterns.
1574-1578: Align balance retrieval test.Using
get_native_and_token_balances(instances.token, addresses.clone())correctly tests single-recipient behavior.
1629-1634: Inline address generation is clear.Pushing random addresses in the loop matches the style used elsewhere.
1649-1653: Balance assertion updated for multi-recipient.Verifying both native and token balances for multiple addresses aligns with the refactored helper.
1669-1674: Populate safes and nodes succinctly.Looping to fill both
safe_addressesandnode_addressesin one place is concise and consistent.
1731-1735: Adjust hex fixture for salt computation.Updated salt value matches the new
get_salt_from_salt_noncelogic and expected test vector.
2349-2358: Call debug helper with updated signature.Passing
module_address,&node_addresses[0], and other addresses directly matches the refactoreddebug_node_safe_module_setup_mainparameters.chain/actions/src/redeem.rs (3)
476-476: LGTM: Unnecessary clone removedGood improvement removing the unnecessary
.clone()call onchannel_from_bob. This aligns with clippy suggestions to avoid redundant cloning when the value can be moved or referenced directly.
766-771: LGTM: Improved iteration patternExcellent improvement replacing the indexed loop with direct iteration over ticket references. This is more idiomatic Rust code and eliminates potential index-out-of-bounds issues. The
.clone()call on line 768 is still necessary sinceredeem_tickettakes ownership of the ticket.
838-838: LGTM: Unnecessary clone removedGood improvement removing the unnecessary
.clone()call onchannel_from_bob, consistent with the similar change on line 476. This reduces unnecessary allocations and follows clippy best practices.common/internal-types/src/tickets.rs (2)
705-705: LGTM: Explicit big-endian conversion improves clarityExcellent improvement using
U256::from_big_endian(&amount)explicitly. This makes the byte order interpretation clear and ensures consistent numeric representation across the codebase, which aligns with similar standardizations mentioned in other modules.
1163-1164: LGTM: Simplified test constructionGood simplification replacing
try_fromconversion with direct struct initialization in test code. This removes unnecessary error handling overhead in tests and makes the code more concise while maintaining the same functionality.chain/indexer/src/handlers.rs (4)
530-533: LGTM! Standardized U256 conversion improves clarity.The explicit use of
U256::from_big_endian()with byte slices makes the big-endian conversion intention clear and consistent with the broader codebase standardization efforts.
564-567: LGTM! Consistent Balance construction pattern.This change maintains consistency with the standardized U256 conversion approach used throughout the codebase.
785-791: LGTM! Consistent with ticket price handling standardization.The standardized Balance construction approach is consistently applied across all event handlers.
1018-1018: LGTM! Excellent test code cleanup removing redundant conversions.These changes remove unnecessary
.into()calls and simplify argument passing in test code, making it cleaner and addressing clippy warnings about redundant conversions. The simplified code is more readable while maintaining the same functionality.Also applies to: 1030-1030, 1044-1044, 1082-1082, 1098-1098, 1149-1149, 1208-1208, 1260-1260, 1265-1265, 1279-1279, 1307-1307, 1313-1313, 1320-1320, 1327-1331, 1346-1353, 1356-1356, 1362-1362, 1369-1369, 1388-1388, 1394-1394, 1405-1405, 1412-1412, 1417-1421, 1424-1424, 1431-1431, 1438-1438, 1452-1452, 1458-1458, 1471-1471, 1494-1494, 1501-1501, 1514-1514, 1542-1542, 1548-1548, 1560-1560, 1588-1588, 1595-1595, 1607-1607, 1632-1632, 1639-1639, 1646-1646, 1666-1666, 1673-1673, 1680-1680, 1698-1698, 1705-1705, 1712-1712, 1736-1736, 1743-1743, 1750-1750, 1780-1780, 1786-1786, 1792-1792, 1799-1799, 1827-1827, 1833-1833, 1842-1842, 1870-1870, 1889-1889, 1895-1895, 1902-1902, 1925-1925, 1941-1941, 1947-1947, 1954-1954, 1986-1986, 2002-2002, 2008-2008, 2015-2015, 2041-2041, 2048-2048, 2055-2055, 2100-2100, 2107-2107, 2114-2114, 2160-2160, 2167-2167, 2173-2173, 2238-2238, 2248-2248, 2272-2272, 2351-2351, 2355-2357, 2381-2381, 2447-2447, 2453-2453, 2460-2460, 2518-2518, 2524-2524, 2531-2531, 2571-2571, 2577-2577, 2584-2584, 2626-2626, 2632-2632, 2640-2640, 2670-2670, 2677-2677, 2684-2684, 2704-2704, 2711-2711, 2718-2718, 2739-2739, 2744-2744, 2754-2754, 2782-2782, 2785-2785, 2797-2797, 2829-2829, 2852-2852, 2875-2875, 2879-2879, 2886-2886
chain/rpc/src/rpc.rs (3)
203-206: LGTM! Standardized balance construction across native and token balances.The consistent use of
U256::from_big_endian()for both native and HOPR token balance construction ensures uniform handling of blockchain-sourced numeric values throughout the RPC operations.Also applies to: 209-212
231-234: LGTM! Consistent U256 conversion pattern extended to all RPC operations.The standardized conversion approach is now consistently applied across ticket prices and module target information, creating a uniform pattern for handling blockchain numeric data.
Also applies to: 321-321
452-452: LGTM! Clean test code improvements.The removal of unnecessary
.into()conversions and code simplifications make the test code cleaner while maintaining identical functionality. These changes address clippy warnings effectively.Also applies to: 582-582
crypto/packet/benches/packet_benches.rs (3)
20-20: LGTM! Optimized public key handling.Replacing
.clone()with dereferencing (*) forOffchainPublicKeyis correct since it implements theCopytrait. This change improves performance by avoiding unnecessary heap allocations.
24-24: LGTM! Consistent optimization across all benchmark functions.The pattern of using
*kinstead ofk.clone()is consistently applied throughout all benchmark functions. This leverages theCopytrait implementation forOffchainPublicKey, making the code more efficient.Also applies to: 73-73, 120-120, 167-167, 220-220, 281-281
260-260: LGTM! Improved public key dereferencing in packet processing.Using
*sender.public()and*relayer.public()instead of cloning is the correct approach forCopytypes. This change eliminates unnecessary allocations in the benchmark hot paths.Also applies to: 316-316, 332-332
common/network-types/src/session/state.rs (4)
1141-1141: LGTM! More idiomatic iterator usage.Replacing
std::iter::repeat(frame_size).take(num_frames)withstd::iter::repeat_n(frame_size, num_frames)is more idiomatic and expresses the intent more clearly. This likely fixes a clippy::iter_nth warning.
1155-1155: LGTM! Explicit result handling.Using
let _ = socket.write(&write).await?;explicitly indicates that the result is intentionally ignored, which is clearer than using.unwrap()and likely fixes a clippy::unused_must_use warning.
1477-1477: LGTM! Improved error handling in test function.Converting the test function to return
anyhow::Result<()>and usingawait?for error propagation is a better practice than using.await.unwrap(). This provides cleaner error handling and likely fixes clippy warnings about panic-prone code in tests.Also applies to: 1494-1494, 1506-1507
1511-1511: LGTM! Consistent error handling pattern.The same error handling improvements are consistently applied to this second test function, maintaining a uniform approach across the test suite.
Also applies to: 1528-1528, 1540-1541
Changes extracted from #7142