GH-686 Limits on rate-pack values #736
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
I am still uncertain, this will protect the MASQ Foundation network, because if someone want's to set rate-pack below the limit, it is just one change and cargo build away from doing so and network does not even aknowledge this. I woud like to propose: make an debut_handler rule, that if the debuting node is in standard, or originate-only mode, and have rate pack below (or above) the limits, his debut is dropped on the flor
|
Hey @dnwiebe @czarte - I agree with @czarte comments to add this additional checking in the handlers to ensure Nodes don't have rate-packs outside of the defined limits now, and can be done without too much additional work. This will cover older versions of Node joining network with rates outside the hard-coded limits |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| @@ -0,0 +1,67 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| @@ -1412,6 +1854,58 @@ impl GossipAcceptorReal { | |||
| debut_target_node_addr.clone(), | |||
| )) | |||
| } | |||
|
|
|||
| fn validate_new_version( | |||
There was a problem hiding this comment.
hint: what about name: validate_standard_nodes_requirements
reason: in gossip we refering with version to NeighborhoodDatabase version, so it seems bit confusing name for me here
There was a problem hiding this comment.
I decided on validate_incoming_agr(); I think that's pretty clear.
| let (mut valid_agrs, mut bans) = so_far; | ||
| if &agr.inner.public_key == database.root_key() { | ||
| // Shouldn't ever happen; but an evil Node could try it | ||
| // valid_agrs.push(agr); |
| Qualification::Malformed(malefactor) => { | ||
| let (public_key_opt, ip_address_opt, earning_wallet_opt) = | ||
| match agrs.iter().find(|agr| { | ||
| agr.node_addr_opt.as_ref().map(|na| na.ip_addr()) |
There was a problem hiding this comment.
Note: I would like to see the tests of malefactor claiming our own ip, that this ip is not comming in here
There was a problem hiding this comment.
In method extract_malefactors it looks like we are using agr ip in case we malefactor him for claiming our own IP
There was a problem hiding this comment.
I don't understand this issue. I'll ask you about it.
| let (gossip, gossip_source) = make_introduction(2345, 3456); | ||
| let dest_root = make_node_record(7878, true); | ||
| let mut dest_db = db_from_node(&dest_root); | ||
| // These don't count because they're half-only neighbors. Will they be ignored? |
There was a problem hiding this comment.
Due to Ban and Log message those are ignored. shout the question stay?
| return; | ||
| } | ||
|
|
||
| trace!(self.logger, "Contents: {}", agrs_to_string(agrs.as_slice())); |
There was a problem hiding this comment.
is this necessary here or is leftover from dev?
| .rate_pack_limits_result(Ok(RatePackLimits::test_default())), | ||
| )); | ||
| subject.data_directory = data_dir; | ||
| // subject.data_directory = data_dir; |
| ); | ||
| subject.data_directory = data_dir.to_path_buf(); | ||
| subject.connect_database(); | ||
| // subject.data_directory = data_dir.to_path_buf(); |
| } | ||
| } | ||
|
|
||
| pub struct PersistentConfigurationFactoryTest { |
There was a problem hiding this comment.
you said, this is Mock, not Test
| } | ||
| } | ||
|
|
||
| impl NodeRecord { |
3 participants
Note
Medium Risk
Introduces a new persisted config value and bumps DB schema to v12 with a migration, which can affect node startup/migrations and rate-pack validation. Remaining changes are primarily test/integration stability tweaks and small refactors/logging.
Overview
Adds rate-pack limits as a first-class persisted setting. Introduces
DEFAULT_RATE_PACK_LIMITS/RatePackLimits, storesrate_pack_limitsin the DB on init, and bumpsCURRENT_SCHEMA_VERSIONto12with a newmigration_11_to_12inserting the default limits.Updates configuration plumbing and tests.
PersistentConfigurationgainsrate_pack_limits()with strict parsing/ordering checks, the daemon’sConfigDaoNullnow supplies a default, and setup-reporting tests update expectedrate-packvalues (scaled to larger wei-like numbers).Stability/refactor cleanup. Multinode tests are made less flaky (timeouts/sleeps, different external test host, clearer mismatch diagnostics), Docker network creation now retries, gossip/dot-graph rendering gets minor API/formatting tweaks, and accountant charging/logging is refactored to compute
total_chargeonce and include it in debug output.Written by Cursor Bugbot for commit 53cadb4. This will update automatically on new commits. Configure here.