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.
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.
| } | ||
|
|
||
| fn create_network() -> Result<(), String> { | ||
| let mut errors = vec![]; |
There was a problem hiding this comment.
Evidently with newer versions of Docker, it can take a little while for a Docker network to be destroyed. Some of our multinode tests were failing because they couldn't create a new network until the network for the previous test completed its destruction. So...this code tries three times to create a new network, once every quarter second.
|
|
||
| // Waiting until everybody has finished generating payables and receivables | ||
| thread::sleep(Duration::from_secs(10)); | ||
|
|
There was a problem hiding this comment.
Moved up from below. It was pretty silly to have it down there and not up here. @czarte noticed this.
|
|
||
| // get all receivables from all other nodes | ||
| let receivable_balances = non_originating_nodes | ||
| let receivable_nodes = non_originating_nodes |
There was a problem hiding this comment.
Somewhat better name. Now we're storing the Node name as well as its balance, so that we can generate better failure messages.
Note
Introduces bounded rate packs and migrates configuration/storage accordingly.
rate_pack_limitsto config with parsing/validation and accessors; implementsPersistentConfigurationFactoryand anInvalidplaceholder11→12inserts defaultrate_pack_limits; bumpsCURRENT_SCHEMA_VERSIONto12rate-packvalues; adjusts UI/setup, config DAO, and initializer to surface defaultstotal_charge) and clarifies debug logs; updates unit testsNodeRenderablefield rename,AccessibleGossipRecordDisplay/formatting, addagrs_to_string; enabletimecratelocal-offsetfeatureWritten by Cursor Bugbot for commit 9313296. This will update automatically on new commits. Configure here.