chore: bump to nearcore 2.13.2 - #3988
Conversation
|
@jancionear here we are facing a problem where some of our tests are timing out after the bump, where before it, nearcore 3.13.1 they were fine. Apparently the problem originated in near/nearcore@df8d1a01 which afaik was originally your code. A quick summary of the diff in test performance is:
Does that make sense to you? If yes, is this something we need to fix on our side, or on nearcore side? Claude mentioned the proper fix is: |
|
Huh that's weird. The rate limits should be high enough to allow the network to run at a normal pace, no matter the load. How do these tests work? Do they run at a normal block time? Or are they sped up to finish more quickly? Speeding it up could go cause the message rate to go above the expected limit. |
Exactly, they are run at a much faster block time than normal |
|
If the tests export normal metrics you could also try to look at these ones: And see how they differ between version bumps or between tests and a real node |
Ah that makes sense. Yeah in that case you can disable the rate limits using the config option that claude mentioned. But it's better not to do it on the production setup, as it makes the node more DoSable |
Yeah that's the plan, thanks! |
|
We could probably add a Although that would go in 2.13.3 or a custom patch |
Pull request overviewBumps the workspace Changes:
Reviewed changesPer-file summary
FindingsBlocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
|
4272ecf to
8e01ddd
Compare
8e01ddd to
f2599b8
Compare
|
@claude review |
Pull request overviewBumps the workspace Since the previous review round the parity claim at Changes:
Reviewed changesPer-file summary
FindingsBlocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
|
| if near_init.chain_id.is_localnet() { | ||
| object_at(config, &[])?.insert( | ||
| "state_sync_enabled".to_owned(), | ||
| serde_json::Value::Bool(false), | ||
| ); | ||
| lift_localnet_block_rate_limits(config)?; | ||
| } else { | ||
| // Decentralized (peer-to-peer) state sync; replaces any inherited ExternalStorage block. | ||
| set_decentralized_state_sync(config)?; | ||
| } |
There was a problem hiding this comment.
Should we add a test for localnet to run WITH the true rate limits as well? I feel like rate limit diffs between local and "prod" is quite significant.
There was a problem hiding this comment.
This is a relatively complicated matter. Afaik the new rate limits are meant to avoid some possible adversarial behaviors, so they shouldn't add a new gap in tests with honest participants (like the ones we have) in any way. The only reason they did affect our tests is that by default these tests are tweaked to make block production go faster.
Closes #3992