Remove bootstrapping retry config #2301

StephenButtolph · 2023-11-14T00:37:41Z

Why this should be merged

We added these configs when we added in bootstrapping retries. I have never seen anyone actually use these configs, and I personally find the retries counters very confusing in the code.
I'm trying to simplify the bootstrapping protocol as much as possible, and this seemed like a clear thing to 🪓.

How this works

Removes the option to disable bootstrapping retries and removes the periodic warning.

How this was tested

CI + syncing fuji locally

config/keys.go

StephenButtolph · 2023-11-16T21:22:19Z

snow/engine/common/bootstrapper.go

@@ -162,22 +158,9 @@ func (b *bootstrapper) Startup(ctx context.Context) error {
 	return b.sendMessagesOrFinish(ctx)
 }

-func (b *bootstrapper) Restart(ctx context.Context, reset bool) error {


Restart(ctx, false) is replaced with Startup(ctx) and Restart(ctx, false) is replaced with Restart(ctx)

snow/engine/snowman/bootstrap/bootstrapper.go

StephenButtolph · 2023-11-16T22:26:18Z

vms/platformvm/vm_test.go

+	externalSender.SendF = func(msg message.OutboundMessage, nodeIDs set.Set[ids.NodeID], _ ids.ID, _ subnets.Allower) set.Set[ids.NodeID] {
+		inMsg, err := mc.Parse(msg.Bytes(), ctx.NodeID, func() {})
+		require.NoError(err)
+		require.Equal(message.GetAcceptedFrontierOp, inMsg.Op())
+
+		requestID, ok := message.GetRequestID(inMsg.Message())
+		require.True(ok)
+
+		reqID = requestID
+		return nodeIDs
+	}
+
+	require.NoError(bootstrapper.Ancestors(context.Background(), peerID, reqID, [][]byte{advanceTimeBlkBytes}))
+
+	externalSender.SendF = func(msg message.OutboundMessage, nodeIDs set.Set[ids.NodeID], _ ids.ID, _ subnets.Allower) set.Set[ids.NodeID] {
+		inMsgIntf, err := mc.Parse(msg.Bytes(), ctx.NodeID, func() {})
+		require.NoError(err)
+		require.Equal(message.GetAcceptedOp, inMsgIntf.Op())
+		inMsg := inMsgIntf.Message().(*p2p.GetAccepted)
+
+		reqID = inMsg.RequestId
+		return nodeIDs
+	}
+
+	require.NoError(bootstrapper.AcceptedFrontier(context.Background(), peerID, reqID, advanceTimeBlkID))


We need to do another round of the bootstrapping protocol because we are correctly restarting the bootstrapper now.

StephenButtolph · 2023-11-16T22:44:16Z

snow/engine/snowman/bootstrap/bootstrapper.go

@@ -587,8 +587,8 @@ func (b *bootstrapper) checkFinish(ctx context.Context) error {
 	// Note that executedBlocks < c*previouslyExecuted ( 0 <= c < 1 ) is enforced
 	// so that the bootstrapping process will terminate even as new blocks are
 	// being issued.
-	if b.Config.RetryBootstrap && executedBlocks > 0 && executedBlocks < previouslyExecuted/2 {
-		return b.Restart(ctx, true)
+	if executedBlocks > 0 && executedBlocks < previouslyExecuted/2 {


This change is why test tests needed to be updated (because b.Config.RetryBootstrap was false in a number of tests previously)

abi87 · 2023-11-17T09:13:24Z

snow/engine/common/bootstrapper.go

@@ -31,7 +31,7 @@ type Bootstrapper interface {
 	AcceptedHandler
 	Haltable
 	Startup(context.Context) error
-	Restart(ctx context.Context, reset bool) error
+	Restart(ctx context.Context) error


It looks like this all common bootstrapper is really only used by snowman chains (not avalanche ones anymore).
Should we just embed it into the snowman bootstrapped and be done with the common thing?
Maybe outside the scope of this PR but we'd prolly simplify code more aggressively

That's the goal!

abi87 · 2023-11-17T10:16:01Z

snow/engine/snowman/bootstrap/bootstrapper_test.go

 	require.Equal(choices.Accepted, blk0.Status())
 	require.Equal(choices.Accepted, blk1.Status())
 	require.Equal(choices.Accepted, blk2.Status())
 	require.Equal(choices.Accepted, blk3.Status())
 	require.Equal(choices.Accepted, blk4.Status())
+
+	require.NoError(bs.ForceAccepted(context.Background(), []ids.ID{blkID4}))


maybe a small comment to easy up UTs maintenance?
Something like that we confirm the latest downlaoded block twice to make sure bootstrap is marked as done.
It could even be made once at the top of this test file

I added the comment to the actual bootstrapper struct

commit 4768ed4 Author: Stephen Buttolph <[email protected]> Date: Fri Nov 17 12:50:07 2023 -0500 Remove bootstrapping retry config (#2301) commit 392b313 Author: Stephen Buttolph <[email protected]> Date: Fri Nov 17 12:43:15 2023 -0500 Move engine startup into helper function (#2329) commit fe72c5b Author: Stephen Buttolph <[email protected]> Date: Fri Nov 17 01:23:21 2023 -0500 Remove `common.Config` functions (#2328) commit 585424e Author: Stephen Buttolph <[email protected]> Date: Fri Nov 17 00:39:46 2023 -0500 Unexport avalanche constant from common package (#2327) commit 8520112 Author: Dhruba Basu <[email protected]> Date: Thu Nov 16 20:53:10 2023 -0800 Move `network` implementation to separate package (#2296) Signed-off-by: Stephen Buttolph <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 5236d72 Author: Stephen Buttolph <[email protected]> Date: Thu Nov 16 20:08:57 2023 -0500 Remove useless anon functions (#2326) commit e7ca38b Author: Dan Laine <[email protected]> Date: Thu Nov 16 16:03:17 2023 -0500 Update zap dependency to v1.26.0 (#2325) commit 6900e72 Author: Dan Laine <[email protected]> Date: Thu Nov 16 15:20:19 2023 -0500 nit: loop --> variadic (#2316) commit 35fbb3a Author: Alberto Benegiamo <[email protected]> Date: Thu Nov 16 10:33:18 2023 -0700 Pchain - Cleanup NodeID generation in UTs (#2291) Co-authored-by: Dan Laine <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 043644f Author: Stephen Buttolph <[email protected]> Date: Thu Nov 16 12:13:09 2023 -0500 Refactor bootstrapper implementation into consensus (#2300) Co-authored-by: Dan Laine <[email protected]> commit f1ec30c Author: Joshua Kim <[email protected]> Date: Thu Nov 16 12:03:00 2023 -0500 Update `error_code` to be int32 instead of uint32. (#2322) Signed-off-by: Joshua Kim <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 348f842 Author: Dhruba Basu <[email protected]> Date: Thu Nov 16 08:39:03 2023 -0800 Remove `Network` interface from `Builder` (#2312) commit 6484de4 Author: Joshua Kim <[email protected]> Date: Thu Nov 16 04:31:34 2023 -0500 Rename AppRequestFailed to AppError (#2321) Signed-off-by: Joshua Kim <[email protected]> commit 3d0611c Author: Joshua Kim <[email protected]> Date: Wed Nov 15 19:24:10 2023 -0500 Remove error from SDK AppGossip handler (#2252) Signed-off-by: Joshua Kim <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 01a1bbe Author: Dhruba Basu <[email protected]> Date: Wed Nov 15 16:01:29 2023 -0800 Remove `AddUnverifiedTx` from `Builder` (#2311) commit e8ef4ad Author: Dhruba Basu <[email protected]> Date: Wed Nov 15 14:15:17 2023 -0800 Move `AddUnverifiedTx` logic to `network.IssueTx` (#2310) commit dcc6ea8 Author: Stephen Buttolph <[email protected]> Date: Wed Nov 15 17:13:19 2023 -0500 Use zap.Stringer rather than zap.Any (#2320) commit 44f3aba Author: Stephen Buttolph <[email protected]> Date: Wed Nov 15 17:12:57 2023 -0500 Replace unique slices with sets in the engine interface (#2317) commit d00b67f Author: Stephen Buttolph <[email protected]> Date: Wed Nov 15 11:43:17 2023 -0500 Simplify avalanche bootstrapping (#2286) commit 5dff153 Author: Dhruba Basu <[email protected]> Date: Tue Nov 14 12:47:06 2023 -0800 Add `VerifyTx` to `executor.Manager` (#2293) commit 29f86e9 Author: marun <[email protected]> Date: Tue Nov 14 21:25:25 2023 +0100 e2e: More fixture refinement in support of coreth integration testing (#2275) commit 72d2fae Author: Dhruba Basu <[email protected]> Date: Tue Nov 14 10:26:39 2023 -0800 Add `recentTxsLock` to platform `network` struct (#2294) commit eb21b42 Author: Dhruba Basu <[email protected]> Date: Tue Nov 14 10:24:01 2023 -0800 Move management of platformvm preferred block to `executor.Manager` (#2292) commit 7f70fcf Author: Stephen Buttolph <[email protected]> Date: Mon Nov 13 15:18:34 2023 -0500 Simplify get server creation (#2285) Co-authored-by: Dan Laine <[email protected]> commit baf0ef7 Author: Dan Laine <[email protected]> Date: Mon Nov 13 10:33:31 2023 -0500 `merkledb` -- Add `Clearer` interface (#2277) commit b8746de Author: Dhruba Basu <[email protected]> Date: Thu Nov 9 16:21:04 2023 -0800 Embed `noop` handler for all unhandled messages (#2288) commit 86201ae Author: David Boehm <[email protected]> Date: Thu Nov 9 14:08:10 2023 -0500 Remove sentinel node from MerkleDB proofs (#2106) Signed-off-by: David Boehm <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> Co-authored-by: Dan Laine <[email protected]> commit 094ce50 Author: Joshua Kim <[email protected]> Date: Thu Nov 9 13:58:28 2023 -0500 Remove Lazy Initialize on Node (#1384) Signed-off-by: Joshua Kim <[email protected]> Co-authored-by: Dan Laine <[email protected]> commit e3f1212 Author: Alberto Benegiamo <[email protected]> Date: Thu Nov 9 10:08:59 2023 -0700 Genesis validators cleanup (#2282) commit 151621f Author: Alberto Benegiamo <[email protected]> Date: Thu Nov 9 09:33:42 2023 -0700 Cleanup `ids.NodeID` usage (#2280) commit ebaf9d4 Author: Dhruba Basu <[email protected]> Date: Wed Nov 8 18:08:23 2023 -0500 Move `DropExpiredStakerTxs` to platformvm mempool (#2279) commit c94ff4e Author: David Boehm <[email protected]> Date: Wed Nov 8 17:55:02 2023 -0500 MerkleDB:Naming and comments cleanup (#2274) Co-authored-by: Dan Laine <[email protected]> commit bcd4a94 Author: Dhruba Basu <[email protected]> Date: Wed Nov 8 17:28:33 2023 -0500 Cleanup platformvm mempool errs (#2278) commit aba404e Author: marun <[email protected]> Date: Wed Nov 8 22:54:42 2023 +0100 e2e: Refactor suite setup and helpers to tests/fixture/e2e for reuse by coreth (#2265) commit fc95834 Author: Dhruba Basu <[email protected]> Date: Wed Nov 8 16:41:39 2023 -0500 `mempool.NewMempool` -> `mempool.New` (#2276) commit 93d88c0 Author: Dhruba Basu <[email protected]> Date: Wed Nov 8 15:02:10 2023 -0500 Return if element was deleted from `Hashmap` (#2271) Co-authored-by: Dan Laine <[email protected]> commit 1329a59 Author: Dan Laine <[email protected]> Date: Wed Nov 8 12:56:27 2023 -0500 Add fuzz test for `NewIteratorWithStartAndPrefix` (#1992) Co-authored-by: Alberto Benegiamo <[email protected]> commit 22f3c89 Author: Dan Laine <[email protected]> Date: Tue Nov 7 19:12:17 2023 -0500 `merkledb` -- remove unneeded var declarations (#2269) commit 683fcfa Author: Dan Laine <[email protected]> Date: Tue Nov 7 17:36:20 2023 -0500 Add read-only database flag (`--db-read-only`) (#2266) commit 52f93c8 Author: Dan Laine <[email protected]> Date: Tue Nov 7 17:34:19 2023 -0500 `merkledb` -- fix nil check in test (#2268) commit 1faec38 Author: Dan Laine <[email protected]> Date: Tue Nov 7 17:34:09 2023 -0500 `merkledb` -- rename nit (#2267) commit cdcfb5b Author: felipemadero <[email protected]> Date: Tue Nov 7 17:06:59 2023 -0300 Use extended public key to derive ledger addresses (#2246) Signed-off-by: felipemadero <[email protected]> Co-authored-by: Dan Laine <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 7490a92 Author: Joshua Kim <[email protected]> Date: Mon Nov 6 15:39:23 2023 -0500 Document p2p package (#2254) Signed-off-by: Joshua Kim <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 10bd428 Author: Dhruba Basu <[email protected]> Date: Mon Nov 6 14:49:45 2023 -0500 Remove unused `UnsortedEquals` function (#2264) commit e710899 Author: David Boehm <[email protected]> Date: Mon Nov 6 13:36:34 2023 -0500 Remove Token constants information from keys (#2197) Signed-off-by: David Boehm <[email protected]> Signed-off-by: Dan Laine <[email protected]> Co-authored-by: Darioush Jalali <[email protected]> Co-authored-by: Dan Laine <[email protected]> commit 558d8fb Author: vuittont60 <[email protected]> Date: Mon Nov 6 22:26:43 2023 +0800 Fix typos in docs (#2261) Signed-off-by: vuittont60 <[email protected]> commit a8db08e Author: marun <[email protected]> Date: Mon Nov 6 15:26:27 2023 +0100 e2e: Make NewWallet and NewEthclient regular functions (#2262) commit aaed8f3 Author: Stephen Buttolph <[email protected]> Date: Sat Nov 4 17:29:12 2023 -0400 Track all subnet validator sets in the validator manager (#2253) commit cec1cd1 Author: Stephen Buttolph <[email protected]> Date: Fri Nov 3 19:19:26 2023 -0400 Require poll metrics to be registered (#2260) commit e4cb2cd Author: Dan Laine <[email protected]> Date: Fri Nov 3 17:33:44 2023 -0400 Cleanup `ipcs` `Socket` test (#2257) commit 437ade8 Author: marun <[email protected]> Date: Fri Nov 3 21:45:12 2023 +0100 Switch to using require.TestingT interface in SenderTest struct (#2258) commit 11f1b55 Author: DoTheBestToGetTheBest <[email protected]> Date: Thu Nov 2 15:41:47 2023 -0700 feat(api) : Peers function to return the PrimaryAlias of the chainID (#2251) Signed-off-by: DoTheBestToGetTheBest <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit c174c62 Author: Stephen Buttolph <[email protected]> Date: Thu Nov 2 16:20:03 2023 -0400 Return log levels from admin.SetLoggerLevel (#2250) commit 20f3580 Author: Stephen Buttolph <[email protected]> Date: Wed Nov 1 22:11:47 2023 -0400 Update versions for v1.10.15 (#2245) commit 36d1630 Author: Cesar <[email protected]> Date: Wed Nov 1 22:44:14 2023 -0300 Add nullable option to codec (#2171) Signed-off-by: Cesar <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> Co-authored-by: Dan Laine <[email protected]> commit 4957ccb Author: Dan Laine <[email protected]> Date: Tue Oct 31 18:38:57 2023 -0400 Add `pebble` as valid value for `--db-type`. (#2244) Signed-off-by: Dan Laine <[email protected]> Co-authored-by: Dhruba Basu <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 047d493 Author: Dhruba Basu <[email protected]> Date: Tue Oct 31 18:14:14 2023 -0400 Add `BaseTx` support to platformvm (#2232) Signed-off-by: Dhruba Basu <[email protected]> commit 1f9df8f Author: Dan Laine <[email protected]> Date: Tue Oct 31 17:10:05 2023 -0400 Remove `database.Manager` (#2239) Signed-off-by: Dan Laine <[email protected]> commit 8d15c22 Author: Stephen Buttolph <[email protected]> Date: Tue Oct 31 13:28:49 2023 -0400 Document host and port behavior in help text (#2236) commit 76d756f Author: Joshua Kim <[email protected]> Date: Tue Oct 31 11:43:13 2023 -0400 Remove error from Router AppGossip (#2238) Signed-off-by: Joshua Kim <[email protected]> commit 5b96789 Author: Joshua Kim <[email protected]> Date: Mon Oct 30 16:41:51 2023 -0400 P2P AppRequestFailed protobuf definition (#2111) Signed-off-by: Joshua Kim <[email protected]> Co-authored-by: Stephen Buttolph <[email protected]> commit 826f941 Author: Dhruba Basu <[email protected]> Date: Fri Oct 27 19:41:19 2023 -0400 Fix test typos (#2233) Signed-off-by: marun <[email protected]> Co-authored-by: marun <[email protected]> commit 66375f5 Author: Dhruba Basu <[email protected]> Date: Fri Oct 27 18:44:34 2023 -0400 Trim down size of secp256k1 `Factory` struct (#2223) Signed-off-by: Dhruba Basu <[email protected]> commit 42d4e3e Author: Dhruba Basu <[email protected]> Date: Fri Oct 27 17:44:38 2023 -0400 Enable `perfsprint` linter (#2229) commit b83af9b Author: Dhruba Basu <[email protected]> Date: Fri Oct 27 17:42:03 2023 -0400 Add `utils.Err` helper (#2212) Signed-off-by: Dhruba Basu <[email protected]> Signed-off-by: Joshua Kim <[email protected]>

Remove bootstrapping retry config

d0cdbf9

StephenButtolph added consensus This involves consensus cleanup Code quality improvement labels Nov 14, 2023

StephenButtolph self-assigned this Nov 14, 2023

StephenButtolph requested review from abi87 and danlaine as code owners November 14, 2023 00:37

StephenButtolph added this to the v1.10.16 milestone Nov 14, 2023

StephenButtolph marked this pull request as draft November 14, 2023 00:42

abi87 reviewed Nov 14, 2023

View reviewed changes

config/keys.go Show resolved Hide resolved

merged

8034b65

StephenButtolph commented Nov 16, 2023

View reviewed changes

snow/engine/snowman/bootstrap/bootstrapper.go Show resolved Hide resolved

fix platformvm test

a3f0eed

StephenButtolph commented Nov 16, 2023

View reviewed changes

fix sm tests

08fcf30

StephenButtolph commented Nov 16, 2023

View reviewed changes

StephenButtolph added 5 commits November 16, 2023 17:46

nit

f53e022

reduce diff

4f3feb6

reduce diff

35bcff2

reduce diff

3647ace

Merge branch 'dev' into remove-bootstrapping-retry-config

1794608

StephenButtolph marked this pull request as ready for review November 16, 2023 22:52

Merge branch 'dev' into remove-bootstrapping-retry-config

47fafb9

StephenButtolph mentioned this pull request Nov 17, 2023

Remove common.Config from syncer.Config #2330

Merged

2 tasks

Merge branch 'dev' into remove-bootstrapping-retry-config

c7cc951

abi87 reviewed Nov 17, 2023

View reviewed changes

abi87 approved these changes Nov 17, 2023

View reviewed changes

add comment

6123fae

dhrubabasu approved these changes Nov 17, 2023

View reviewed changes

StephenButtolph added this pull request to the merge queue Nov 17, 2023

Merged via the queue into dev with commit 4768ed4 Nov 17, 2023
16 checks passed

StephenButtolph deleted the remove-bootstrapping-retry-config branch November 17, 2023 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove bootstrapping retry config #2301

Remove bootstrapping retry config #2301

StephenButtolph commented Nov 14, 2023 •

edited

Loading

StephenButtolph Nov 16, 2023

StephenButtolph Nov 16, 2023 •

edited

Loading

StephenButtolph Nov 16, 2023

abi87 Nov 17, 2023

StephenButtolph Nov 17, 2023

abi87 Nov 17, 2023

StephenButtolph Nov 17, 2023

Remove bootstrapping retry config #2301

Remove bootstrapping retry config #2301

Conversation

StephenButtolph commented Nov 14, 2023 • edited Loading

Why this should be merged

How this works

How this was tested

StephenButtolph Nov 16, 2023

Choose a reason for hiding this comment

StephenButtolph Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

StephenButtolph Nov 16, 2023

Choose a reason for hiding this comment

abi87 Nov 17, 2023

Choose a reason for hiding this comment

StephenButtolph Nov 17, 2023

Choose a reason for hiding this comment

abi87 Nov 17, 2023

Choose a reason for hiding this comment

StephenButtolph Nov 17, 2023

Choose a reason for hiding this comment

StephenButtolph commented Nov 14, 2023 •

edited

Loading

StephenButtolph Nov 16, 2023 •

edited

Loading