GH-686 Don't allow extreme rate packs; reject Gossip about extreme rate packs #745

dnwiebe · 2025-11-17T01:57:49Z

Note

Introduces bounded rate packs and migrates configuration/storage accordingly.

Adds rate_pack_limits to config with parsing/validation and accessors; implements PersistentConfigurationFactory and an Invalid placeholder
New DB migration 11→12 inserts default rate_pack_limits; bumps CURRENT_SCHEMA_VERSION to 12
Updates setup/tests to higher rate-pack values; adjusts UI/setup, config DAO, and initializer to surface defaults
Refactors accountant charge handling (precompute total_charge) and clarifies debug logs; updates unit tests
Multinode test robustness: retry Docker network creation, longer timeouts/sleeps, more nodes; switches HTTP checks to testingmcafeesites.com
Minor API/UX tweaks: NodeRenderable field rename, AccessibleGossipRecord Display/formatting, add agrs_to_string; enable time crate local-offset feature

^{Written by Cursor Bugbot for commit 9313296. This will update automatically on new commits. Configure here.}

cursor

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.

cursor · 2025-12-02T13:19:29Z

node/src/neighborhood/gossip.rs

+                let pieces = vec![
+                    &inner_string[0..index_of_space_after_pk],
+                    addr_string.as_str(),
+                    &inner_string[index_of_space_after_pk + 1..],


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.

node/src/db_config/persistent_configuration.rs

…s now

cursor

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.

cursor · 2026-01-14T12:10:46Z

multinode_integration_tests/tests/min_hops_tests.rs

-    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);


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.

dnwiebe · 2026-01-19T02:11:31Z

multinode_integration_tests/src/masq_node_cluster.rs

    }

    fn create_network() -> Result<(), String> {
+        let mut errors = vec![];


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.

dnwiebe · 2026-01-19T02:12:14Z

multinode_integration_tests/tests/bookkeeping_test.rs


+    // Waiting until everybody has finished generating payables and receivables
+    thread::sleep(Duration::from_secs(10));
+


Moved up from below. It was pretty silly to have it down there and not up here. @czarte noticed this.

dnwiebe · 2026-01-19T02:13:07Z

multinode_integration_tests/tests/bookkeeping_test.rs

-
    // get all receivables from all other nodes
-    let receivable_balances = non_originating_nodes
+    let receivable_nodes = non_originating_nodes


Somewhat better name. Now we're storing the Node name as well as its balance, so that we can generate better failure messages.

dnwiebe · 2026-01-19T03:35:53Z

node/src/accountant/mod.rs

        &self,
        service_rate: u64,
        byte_rate: u64,
+        total_charge: u128,


I wanted to log this in the caller, so I decided to calculate it there and pass it in, rather than calculating it both there and here.

dnwiebe · 2026-01-19T03:39:20Z

node/src/daemon/setup_reporter.rs

            ("neighbors", "masq://base-sepolia:[email protected]:1234,masq://base-sepolia:[email protected]:5678", Set),
            ("payment-thresholds","1234|50000|1000|1000|20000|20000",Set),
-            ("rate-pack","1|3|3|8",Set),
+            ("rate-pack","100|300|300|800",Set),


We have to plump this up to get it above the new minimum.

dnwiebe · 2026-01-19T03:43:11Z

node/src/database/db_initializer.rs

        );
+        Self::set_config_value(
+            conn,
+            "rate_pack_limits",


This is the new value in the CONFIG table in CURRENT_SCHEMA_VERSION 12.

dnwiebe · 2026-01-19T04:31:47Z

node/src/db_config/persistent_configuration.rs


+lazy_static! {
+    static ref RATE_PACK_LIMIT_FORMAT: Regex =
+        Regex::new(r"^(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})\|(\d{1,19})-(\d{1,19})$").unwrap();


The eight limits are stored in the database in a fairly complex string format.

dnwiebe · 2026-01-19T04:36:36Z

node/src/db_config/persistent_configuration.rs

+    }
+}
+
+pub struct PersistentConfigurationInvalid {}


Suddenly we have to have a PersistentConfiguration object much earlier in the bootup sequence than we did before--earlier than the point at which we have the database connected. So...this is a PersistentConfiguration object that can satisfy the trait, but that panics whenever you try to read or write it. Once the database comes online, this implementation is swapped for a real one that's connected to the database.

dnwiebe · 2026-01-19T04:41:50Z

node/src/neighborhood/dot_graph.rs

    pub inner: Option<NodeRenderableInner>,
    pub public_key: PublicKey,
-    pub node_addr: Option<NodeAddr>,
+    pub node_addr_opt: Option<NodeAddr>,


Boyscouting: Long-needed name correction

dnwiebe · 2026-01-19T14:58:06Z

node/src/neighborhood/gossip_acceptor.rs

    // Gossip was ignored because it was evil: ban the sender of the Gossip as a malefactor.
-    Ban(String),
+    Ban(Malefactor),


There are three parts to banning malefactors.

Once identified, gather information about them.

Store the information for future use

When they show up again, re-identify them and ignore whatever they say.

We still don't have parts 2 and 3, but Malefactor is the way we do 1.

dnwiebe added 11 commits October 27, 2025 22:49

Functionality and tests. What's the big picture now?

bb8e699

I think that might be it

61d70f5

Review issues

6f57d99

Starting Node validation

cad907e

One test failing

84e31c5

First Standard Gossip test passing

0fc232b

Some tests are passing. Are all tests passing?

cf83017

Do the multinode tests pass in Actions?

a0a4503

Formatting

c33fe6d

Adjusted Gossip handling. Multinode tests don't always work.

c84ef09

Now Pass Gossip must contain a Node that accepts connections

47b6353

cursor bot reviewed Dec 2, 2025

View reviewed changes

dnwiebe added 3 commits December 8, 2025 21:42

GossipAcceptor unit tests pass

45e87b6

Just installed Sweep; haven't used yet

c5374e6

provided_and_consumed_services_are_recorded_in_databases seems to pas…

0a72126

…s now

cursor bot reviewed Jan 14, 2026

View reviewed changes

dnwiebe added 3 commits January 17, 2026 22:43

Multinode tests passing

b6d4de2

All tests pass

3735030

Merge branch 'master' into GH-686-gossip

9313296

dnwiebe commented Jan 19, 2026

View reviewed changes


		// Waiting until everybody has finished generating payables and receivables
		thread::sleep(Duration::from_secs(10));

GH-686 Don't allow extreme rate packs; reject Gossip about extreme rate packs #745

Are you sure you want to change the base?

GH-686 Don't allow extreme rate packs; reject Gossip about extreme rate packs #745

Uh oh!

Conversation

dnwiebe commented Nov 17, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

cursor bot Dec 2, 2025

Choose a reason for hiding this comment

Bug: Slice bounds panic when no space found in string

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

cursor bot Jan 14, 2026

Choose a reason for hiding this comment

Tests for OneHop and TwoHops routing are disabled

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnwiebe Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dnwiebe commented Nov 17, 2025 •

edited by cursor bot

Loading

dnwiebe Jan 19, 2026 •

edited

Loading