feat: increase default min-retain-blocks to 100_000#6949
feat: increase default min-retain-blocks to 100_000#6949rach-id wants to merge 1 commit intocelestiaorg:mainfrom
Conversation
Increase the default from 3000 to 100_000 to ensure bridge nodes can sync from consensus nodes using the default configuration. Closes celestiaorg#6947 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
In order to confirm this works, @walldiss do you remember in which setup the bridge node wasn't able to sync? would like to test it and see before I merge this PR |
ninabarbakadze
left a comment
ninabarbakadze
left a comment
There was a problem hiding this comment.
lgtm as long as we manually verify
|
turning to draft to avoid merging accidentally |
| // MinRetainBlocks is the minimum number of blocks to retain for state sync. | ||
| // This ensures all blocks in the snapshot window (SnapshotInterval × SnapshotKeepRecent) | ||
| // are retained so other nodes can sync from snapshots. | ||
| MinRetainBlocks uint64 = 3000 |
There was a problem hiding this comment.
[question] If this constant is now unused, can we delete?
| // DefaultMinRetainBlocks is the default number of blocks to retain in the | ||
| // block store. This value is large enough for bridge nodes to sync while | ||
| // still bounding block store growth for operators who don't change the default. | ||
| DefaultMinRetainBlocks uint64 = 100_000 |
There was a problem hiding this comment.
100,000 blocks * 6 seconds per block = 600,000 seconds / 60 seconds per minute / 60 minutes per hour = 166.6666666667 hours = 6.9166666667 days
- Do we want this to be a bit higher than 7 days as a buffer in case blocks avg less than 6 seconds?
- Do we want to add a unit test or follow-up issue to modify this for v9 because we will reduce the block time to 3 seconds so this will have to double to remain ~7 days
There was a problem hiding this comment.
Very good idea 👍 once we test this, I'll address this feedback
|
I tested this in a local node with the
and it worked everytime without errors. cc @walldiss |
3 participants
Summary
DefaultMinRetainBlocks = 100_000const toappconstsand use it as the default inDefaultAppConfig()Closes #6947
Test plan
TestDefaultAppConfigupdated and passes🤖 Generated with Claude Code