GH-686 Don't allow extreme rate packs; reject Gossip about extreme rate packs #745
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| let pieces = vec![ | ||
| &inner_string[0..index_of_space_after_pk], | ||
| addr_string.as_str(), | ||
| &inner_string[index_of_space_after_pk + 1..], |
There was a problem hiding this comment.
Bug: Slice bounds panic when no space found in string
The Display implementation for AccessibleGossipRecord searches for a space character starting at position 9 in inner_string. If no space is found before the end of the string, the while loop exits with index_of_space_after_pk equal to inner_string.len(). The subsequent slice &inner_string[index_of_space_after_pk + 1..] would then try to create a slice starting at len + 1, causing an index out-of-bounds panic. The code assumes a space always exists but doesn't verify one was actually found before using the index.
| candidate.hi.exit_service_rate, | ||
| "exit_service_rate", | ||
| ); | ||
| Ok(candidate) |
There was a problem hiding this comment.
Bug: Rate pack limits panics instead of returning errors
The rate_pack_limits() function returns Result<RatePackLimits, PersistentConfigError> but panics on syntax errors and invalid limit ordering instead of returning errors. This is inconsistent with similar functions like rate_pack(), payment_thresholds(), and scan_intervals() that use combined_params_get_method and properly return parsing errors via the Result type. The syntax error handling via unwrap_or_else(|| panic!(...)) and the validation in check_rate_pack_limit_order that panics on low >= high both violate the function's contract of returning errors.
Additional Locations (1)
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| assert_http_end_to_end_routing(Hops::OneHop); | ||
| assert_http_end_to_end_routing(Hops::TwoHops); | ||
| // assert_http_end_to_end_routing(Hops::OneHop); | ||
| // assert_http_end_to_end_routing(Hops::TwoHops); |
There was a problem hiding this comment.
Tests for OneHop and TwoHops routing are disabled
Medium Severity
Two test assertions for Hops::OneHop and Hops::TwoHops routing are commented out in data_can_be_routed_using_different_min_hops, leaving only Hops::SixHops tested. The PR title indicates this change is about rate pack validation, not test disabling. While the existing comment mentions timeout issues, the solution appears to have been adjusting the node count formula and sleep duration for SixHops, but the other two hop variants were left commented out instead of receiving similar adjustments. This reduces test coverage for shorter routing paths.
Note
Adds guardrails and observability around pricing while improving test robustness.
12; new migration11→12inserts defaultrate_pack_limitsand defaults wired into init and null DAOPersistentConfiguration: parses/validatesrate_pack_limits; new factory/invalid helperstotal_chargeonce, pass explicitly, and log exact wei charged; updates unit testsAccessibleGossipRecordDisplayimpl andagrs_to_stringfor clearer diagnosticstimecratelocal-offsetfeature; setup reporter tests use scaledrate-packvalues; minor bootstrapper/test refactors; repo docs/configs added (SWEEP.md, IDE ignores)Written by Cursor Bugbot for commit 0a72126. This will update automatically on new commits. Configure here.