From f546ca45621061c0058887cd248cd020065cd7f9 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Fri, 1 Mar 2024 17:34:24 -0500 Subject: [PATCH 01/11] Remove double spaces (#2802) --- nat/upnp.go | 2 +- vms/avm/fxs/fx.go | 2 +- vms/platformvm/state/state_test.go | 2 +- wallet/chain/p/builder_test.go | 2 +- wallet/chain/x/builder_test.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nat/upnp.go b/nat/upnp.go index aa26d6d82fc6..d1aab02398b3 100644 --- a/nat/upnp.go +++ b/nat/upnp.go @@ -161,7 +161,7 @@ func getUPnPClient(client goupnp.ServiceClient) upnpClient { } } -// discover() tries to find gateway device +// discover() tries to find gateway device func discover(target string) *upnpRouter { devs, err := goupnp.DiscoverDevices(target) if err != nil { diff --git a/vms/avm/fxs/fx.go b/vms/avm/fxs/fx.go index 2749ee4500a3..7dec9fdfb531 100644 --- a/vms/avm/fxs/fx.go +++ b/vms/avm/fxs/fx.go @@ -46,7 +46,7 @@ type Fx interface { // VerifyOperation verifies that the specified transaction can spend the // provided utxos conditioned on the result being restricted to the provided // outputs. If the transaction can't spend the output based on the input and - // credential, a non-nil error should be returned. + // credential, a non-nil error should be returned. VerifyOperation(tx, op, cred interface{}, utxos []interface{}) error } diff --git a/vms/platformvm/state/state_test.go b/vms/platformvm/state/state_test.go index 0c17fc31a31f..f4e29ccceda6 100644 --- a/vms/platformvm/state/state_test.go +++ b/vms/platformvm/state/state_test.go @@ -317,7 +317,7 @@ func TestPersistStakers(t *testing.T) { r.ErrorIs(err, database.ErrNotFound) }, checkDiffs: func(r *require.Assertions, s *state, staker *Staker, height uint64) { - // pending validators weight diff and bls diffs are not stored + // pending validators weight diff and bls diffs are not stored _, err := s.flatValidatorWeightDiffsDB.Get(marshalDiffKey(staker.SubnetID, height, staker.NodeID)) r.ErrorIs(err, database.ErrNotFound) diff --git a/wallet/chain/p/builder_test.go b/wallet/chain/p/builder_test.go index 473103147549..c971b0200b02 100644 --- a/wallet/chain/p/builder_test.go +++ b/wallet/chain/p/builder_test.go @@ -663,7 +663,7 @@ func makeTestUTXOs(utxosKey *secp256k1.PrivateKey) []*avax.UTXO { utxosAddr := utxosKey.Address() return []*avax.UTXO{ - { // a small UTXO first, which should not be enough to pay fees + { // a small UTXO first, which should not be enough to pay fees UTXOID: avax.UTXOID{ TxID: ids.Empty.Prefix(utxosOffset), OutputIndex: uint32(utxosOffset), diff --git a/wallet/chain/x/builder_test.go b/wallet/chain/x/builder_test.go index f4eb916b693c..0e349ad28565 100644 --- a/wallet/chain/x/builder_test.go +++ b/wallet/chain/x/builder_test.go @@ -461,7 +461,7 @@ func makeTestUTXOs(utxosKey *secp256k1.PrivateKey) []*avax.UTXO { const utxosOffset uint64 = 2024 return []*avax.UTXO{ // currently, the wallet scans UTXOs in the order provided here - { // a small UTXO first, which should not be enough to pay fees + { // a small UTXO first, which should not be enough to pay fees UTXOID: avax.UTXOID{ TxID: ids.Empty.Prefix(utxosOffset), OutputIndex: uint32(utxosOffset), From 11372a43e948dd238dd67e8cfec10755c0be5e58 Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Mon, 4 Mar 2024 05:20:47 -0500 Subject: [PATCH 02/11] [vms/platformvm] Remove `platform.getMaxStakeAmount` (#2795) --- vms/platformvm/client.go | 24 ---------------- vms/platformvm/service.go | 59 --------------------------------------- 2 files changed, 83 deletions(-) diff --git a/vms/platformvm/client.go b/vms/platformvm/client.go index c5457d5009c0..a2720e417b80 100644 --- a/vms/platformvm/client.go +++ b/vms/platformvm/client.go @@ -116,19 +116,6 @@ type Client interface { GetMinStake(ctx context.Context, subnetID ids.ID, options ...rpc.Option) (uint64, uint64, error) // GetTotalStake returns the total amount (in nAVAX) staked on the network GetTotalStake(ctx context.Context, subnetID ids.ID, options ...rpc.Option) (uint64, error) - // GetMaxStakeAmount returns the maximum amount of nAVAX staking to the named - // node during the time period. - // - // Deprecated: The MaxStakeAmount should be calculated using - // GetCurrentValidators, and GetPendingValidators. - GetMaxStakeAmount( - ctx context.Context, - subnetID ids.ID, - nodeID ids.NodeID, - startTime uint64, - endTime uint64, - options ...rpc.Option, - ) (uint64, error) // GetRewardUTXOs returns the reward UTXOs for a transaction // // Deprecated: GetRewardUTXOs should be fetched from a dedicated indexer. @@ -516,17 +503,6 @@ func (c *client) GetTotalStake(ctx context.Context, subnetID ids.ID, options ... return uint64(amount), err } -func (c *client) GetMaxStakeAmount(ctx context.Context, subnetID ids.ID, nodeID ids.NodeID, startTime, endTime uint64, options ...rpc.Option) (uint64, error) { - res := &GetMaxStakeAmountReply{} - err := c.requester.SendRequest(ctx, "platform.getMaxStakeAmount", &GetMaxStakeAmountArgs{ - SubnetID: subnetID, - NodeID: nodeID, - StartTime: json.Uint64(startTime), - EndTime: json.Uint64(endTime), - }, res, options...) - return uint64(res.Amount), err -} - func (c *client) GetRewardUTXOs(ctx context.Context, args *api.GetTxArgs, options ...rpc.Option) ([][]byte, error) { res := &GetRewardUTXOsReply{} err := c.requester.SendRequest(ctx, "platform.getRewardUTXOs", args, res, options...) diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index b3e686e61783..5addf50a3879 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -37,7 +37,6 @@ import ( "github.com/ava-labs/avalanchego/vms/platformvm/status" "github.com/ava-labs/avalanchego/vms/platformvm/txs" "github.com/ava-labs/avalanchego/vms/platformvm/txs/builder" - "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" "github.com/ava-labs/avalanchego/vms/secp256k1fx" avajson "github.com/ava-labs/avalanchego/utils/json" @@ -62,8 +61,6 @@ var ( errPrimaryNetworkIsNotASubnet = errors.New("the primary network isn't a subnet") errNoAddresses = errors.New("no addresses provided") errMissingBlockchainID = errors.New("argument 'blockchainID' not given") - errStartAfterEndTime = errors.New("start time must be before end time") - errStartTimeInThePast = errors.New("start time in the past") ) // Service defines the API calls that can be made to the platform chain @@ -1712,62 +1709,6 @@ func (s *Service) GetTotalStake(_ *http.Request, args *GetTotalStakeArgs, reply return nil } -// GetMaxStakeAmountArgs is the request for calling GetMaxStakeAmount. -type GetMaxStakeAmountArgs struct { - SubnetID ids.ID `json:"subnetID"` - NodeID ids.NodeID `json:"nodeID"` - StartTime avajson.Uint64 `json:"startTime"` - EndTime avajson.Uint64 `json:"endTime"` -} - -// GetMaxStakeAmountReply is the response from calling GetMaxStakeAmount. -type GetMaxStakeAmountReply struct { - Amount avajson.Uint64 `json:"amount"` -} - -// GetMaxStakeAmount returns the maximum amount of nAVAX staking to the named -// node during the time period. -func (s *Service) GetMaxStakeAmount(_ *http.Request, args *GetMaxStakeAmountArgs, reply *GetMaxStakeAmountReply) error { - s.vm.ctx.Log.Debug("deprecated API called", - zap.String("service", "platform"), - zap.String("method", "getMaxStakeAmount"), - ) - - startTime := time.Unix(int64(args.StartTime), 0) - endTime := time.Unix(int64(args.EndTime), 0) - - if startTime.After(endTime) { - return errStartAfterEndTime - } - - s.vm.ctx.Lock.Lock() - defer s.vm.ctx.Lock.Unlock() - - now := s.vm.state.GetTimestamp() - if startTime.Before(now) { - return errStartTimeInThePast - } - - staker, err := executor.GetValidator(s.vm.state, args.SubnetID, args.NodeID) - if err == database.ErrNotFound { - return nil - } - if err != nil { - return err - } - - if startTime.After(staker.EndTime) { - return nil - } - if endTime.Before(staker.StartTime) { - return nil - } - - maxStakeAmount, err := executor.GetMaxWeight(s.vm.state, staker, startTime, endTime) - reply.Amount = avajson.Uint64(maxStakeAmount) - return err -} - // GetRewardUTXOsReply defines the GetRewardUTXOs replies returned from the API type GetRewardUTXOsReply struct { // Number of UTXOs returned From 90a2b13c22116a87670bf05a006401000fe304a6 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 5 Mar 2024 14:26:57 -0500 Subject: [PATCH 03/11] Remove unused engine interface (#2811) --- chains/manager.go | 4 +-- snow/engine/snowman/engine.go | 22 -------------- snow/engine/snowman/test_engine.go | 44 ---------------------------- snow/engine/snowman/traced_engine.go | 42 -------------------------- snow/engine/snowman/transitive.go | 4 +-- 5 files changed, 4 insertions(+), 112 deletions(-) delete mode 100644 snow/engine/snowman/engine.go delete mode 100644 snow/engine/snowman/test_engine.go delete mode 100644 snow/engine/snowman/traced_engine.go diff --git a/chains/manager.go b/chains/manager.go index 8d8ce2a8f150..67810f8567f8 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -847,7 +847,7 @@ func (m *manager) createAvalancheChain( } if m.TracingEnabled { - snowmanEngine = smeng.TraceEngine(snowmanEngine, m.Tracer) + snowmanEngine = common.TraceEngine(snowmanEngine, m.Tracer) } // create bootstrap gear @@ -1194,7 +1194,7 @@ func (m *manager) createSnowmanChain( } if m.TracingEnabled { - engine = smeng.TraceEngine(engine, m.Tracer) + engine = common.TraceEngine(engine, m.Tracer) } // create bootstrap gear diff --git a/snow/engine/snowman/engine.go b/snow/engine/snowman/engine.go deleted file mode 100644 index b5e3fb1020e3..000000000000 --- a/snow/engine/snowman/engine.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package snowman - -import ( - "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/snow/engine/snowman/block" -) - -// Engine describes the events that can occur to a Snowman instance. -// -// The engine is used to fetch, order, and decide on the fate of blocks. This -// engine runs the leaderless version of the Snowman consensus protocol. -// Therefore, the liveness of this protocol tolerant to O(sqrt(n)) Byzantine -// Nodes where n is the number of nodes in the network. Therefore, this protocol -// should only be run in a Crash Fault Tolerant environment, or in an -// environment where lose of liveness and manual intervention is tolerable. -type Engine interface { - common.Engine - block.Getter -} diff --git a/snow/engine/snowman/test_engine.go b/snow/engine/snowman/test_engine.go deleted file mode 100644 index eada8463a041..000000000000 --- a/snow/engine/snowman/test_engine.go +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package snowman - -import ( - "context" - "errors" - - "github.com/stretchr/testify/require" - - "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow/consensus/snowman" - "github.com/ava-labs/avalanchego/snow/engine/common" -) - -var ( - _ Engine = (*EngineTest)(nil) - - errGetBlock = errors.New("unexpectedly called GetBlock") -) - -// EngineTest is a test engine -type EngineTest struct { - common.EngineTest - - CantGetBlock bool - GetBlockF func(context.Context, ids.ID) (snowman.Block, error) -} - -func (e *EngineTest) Default(cant bool) { - e.EngineTest.Default(cant) - e.CantGetBlock = false -} - -func (e *EngineTest) GetBlock(ctx context.Context, blkID ids.ID) (snowman.Block, error) { - if e.GetBlockF != nil { - return e.GetBlockF(ctx, blkID) - } - if e.CantGetBlock && e.T != nil { - require.FailNow(e.T, errGetBlock.Error()) - } - return nil, errGetBlock -} diff --git a/snow/engine/snowman/traced_engine.go b/snow/engine/snowman/traced_engine.go deleted file mode 100644 index e2306dcd1349..000000000000 --- a/snow/engine/snowman/traced_engine.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package snowman - -import ( - "context" - - "go.opentelemetry.io/otel/attribute" - - "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow/consensus/snowman" - "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/trace" - - oteltrace "go.opentelemetry.io/otel/trace" -) - -var _ Engine = (*tracedEngine)(nil) - -type tracedEngine struct { - common.Engine - engine Engine - tracer trace.Tracer -} - -func TraceEngine(engine Engine, tracer trace.Tracer) Engine { - return &tracedEngine{ - Engine: common.TraceEngine(engine, tracer), - engine: engine, - tracer: tracer, - } -} - -func (e *tracedEngine) GetBlock(ctx context.Context, blkID ids.ID) (snowman.Block, error) { - ctx, span := e.tracer.Start(ctx, "tracedEngine.GetBlock", oteltrace.WithAttributes( - attribute.Stringer("blkID", blkID), - )) - defer span.End() - - return e.engine.GetBlock(ctx, blkID) -} diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index de39295e71d0..c7b4348f41f4 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -42,9 +42,9 @@ const ( putGossipPeriod = 10 ) -var _ Engine = (*Transitive)(nil) +var _ common.Engine = (*Transitive)(nil) -func New(config Config) (Engine, error) { +func New(config Config) (common.Engine, error) { return newTransitive(config) } From 4c4bfaa452017ef6d7331850300bd5b5fd86c305 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Tue, 5 Mar 2024 18:23:57 -0500 Subject: [PATCH 04/11] Cleanup Duplicate Transitive Constructor (#2812) Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 6 ++++-- snow/engine/snowman/transitive.go | 6 +----- snow/engine/snowman/transitive_test.go | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 67810f8567f8..89dc69c17c7a 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -841,7 +841,8 @@ func (m *manager) createAvalancheChain( Params: consensusParams, Consensus: snowmanConsensus, } - snowmanEngine, err := smeng.New(snowmanEngineConfig) + var snowmanEngine common.Engine + snowmanEngine, err = smeng.New(snowmanEngineConfig) if err != nil { return nil, fmt.Errorf("error initializing snowman engine: %w", err) } @@ -1188,7 +1189,8 @@ func (m *manager) createSnowmanChain( Consensus: consensus, PartialSync: m.PartialSyncPrimaryNetwork && ctx.ChainID == constants.PlatformChainID, } - engine, err := smeng.New(engineConfig) + var engine common.Engine + engine, err = smeng.New(engineConfig) if err != nil { return nil, fmt.Errorf("error initializing snowman engine: %w", err) } diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index c7b4348f41f4..b446401a88c7 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -44,10 +44,6 @@ const ( var _ common.Engine = (*Transitive)(nil) -func New(config Config) (common.Engine, error) { - return newTransitive(config) -} - func cachedBlockSize(_ ids.ID, blk snowman.Block) int { return ids.IDLen + len(blk.Bytes()) + constants.PointerOverhead } @@ -106,7 +102,7 @@ type Transitive struct { errs wrappers.Errs } -func newTransitive(config Config) (*Transitive, error) { +func New(config Config) (*Transitive, error) { config.Ctx.Log.Info("initializing consensus engine") nonVerifiedCache, err := metercacher.New[ids.ID, snowman.Block]( diff --git a/snow/engine/snowman/transitive_test.go b/snow/engine/snowman/transitive_test.go index dcae2e26c9f6..944e0f2a24e7 100644 --- a/snow/engine/snowman/transitive_test.go +++ b/snow/engine/snowman/transitive_test.go @@ -87,7 +87,7 @@ func setup(t *testing.T, engCfg Config) (ids.NodeID, validators.Manager, *common } } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -377,7 +377,7 @@ func TestEngineMultipleQuery(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -796,7 +796,7 @@ func TestVoteCanceling(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -877,7 +877,7 @@ func TestEngineNoQuery(t *testing.T) { engCfg.VM = vm - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -930,7 +930,7 @@ func TestEngineNoRepollQuery(t *testing.T) { engCfg.VM = vm - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -1630,7 +1630,7 @@ func TestEngineAggressivePolling(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -1732,7 +1732,7 @@ func TestEngineDoubleChit(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -1831,7 +1831,7 @@ func TestEngineBuildBlockLimit(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -2861,7 +2861,7 @@ func TestEngineApplyAcceptedFrontierInQueryFailed(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) @@ -2969,7 +2969,7 @@ func TestEngineRepollsMisconfiguredSubnet(t *testing.T) { return gBlk, nil } - te, err := newTransitive(engCfg) + te, err := New(engCfg) require.NoError(err) require.NoError(te.Start(context.Background(), 0)) From 6c760983499b5f09228e107a74e729c0b746172f Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 5 Mar 2024 20:30:40 -0500 Subject: [PATCH 05/11] Update minimum golang version to v1.21.8 (#2814) --- .github/workflows/build-linux-binaries.yml | 2 +- .github/workflows/build-macos-release.yml | 2 +- .github/workflows/build-public-ami.yml | 2 +- .github/workflows/build-ubuntu-amd64-release.yml | 2 +- .github/workflows/build-ubuntu-arm64-release.yml | 2 +- .github/workflows/build-win-release.yml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/fuzz.yml | 2 +- .github/workflows/fuzz_merkledb.yml | 2 +- CONTRIBUTING.md | 2 +- Dockerfile | 2 +- README.md | 2 +- proto/Dockerfile.buf | 2 +- scripts/build_avalanche.sh | 2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build-linux-binaries.yml b/.github/workflows/build-linux-binaries.yml index 857ba374a20e..2f6f73b50be3 100644 --- a/.github/workflows/build-linux-binaries.yml +++ b/.github/workflows/build-linux-binaries.yml @@ -11,7 +11,7 @@ on: - "*" env: - go_version: '~1.21.7' + go_version: '~1.21.8' jobs: build-x86_64-binaries-tarball: diff --git a/.github/workflows/build-macos-release.yml b/.github/workflows/build-macos-release.yml index ea057dbfadb2..05616745cbd6 100644 --- a/.github/workflows/build-macos-release.yml +++ b/.github/workflows/build-macos-release.yml @@ -26,7 +26,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true - run: go version diff --git a/.github/workflows/build-public-ami.yml b/.github/workflows/build-public-ami.yml index e32e43de235a..3ecd2fd4fb8f 100644 --- a/.github/workflows/build-public-ami.yml +++ b/.github/workflows/build-public-ami.yml @@ -19,7 +19,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true - run: go version diff --git a/.github/workflows/build-ubuntu-amd64-release.yml b/.github/workflows/build-ubuntu-amd64-release.yml index 86be909be5c4..73faff76a6c2 100644 --- a/.github/workflows/build-ubuntu-amd64-release.yml +++ b/.github/workflows/build-ubuntu-amd64-release.yml @@ -11,7 +11,7 @@ on: - "*" env: - go_version: '~1.21.7' + go_version: '~1.21.8' jobs: build-jammy-amd64-package: diff --git a/.github/workflows/build-ubuntu-arm64-release.yml b/.github/workflows/build-ubuntu-arm64-release.yml index 6c0b37d6924d..5bdaf0c62288 100644 --- a/.github/workflows/build-ubuntu-arm64-release.yml +++ b/.github/workflows/build-ubuntu-arm64-release.yml @@ -11,7 +11,7 @@ on: - "*" env: - go_version: '~1.21.7' + go_version: '~1.21.8' jobs: build-jammy-arm64-package: diff --git a/.github/workflows/build-win-release.yml b/.github/workflows/build-win-release.yml index ef4ef29f2fe3..c40366e3ee61 100644 --- a/.github/workflows/build-win-release.yml +++ b/.github/workflows/build-win-release.yml @@ -26,7 +26,7 @@ jobs: - uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true - run: go version diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88c803ffe85c..f1dfd862d566 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ concurrency: cancel-in-progress: true env: - go_version: '~1.21.7' + go_version: '~1.21.8' tmpnet_data_path: ~/.tmpnet/networks/1000 jobs: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 85a5e059b332..52eb42989f3f 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -45,7 +45,7 @@ jobs: - name: Setup Golang uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true # Initializes the CodeQL tools for scanning. diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 07c5d2dbfcb4..58b52a8aa94e 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -18,7 +18,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true - name: Run fuzz tests shell: bash diff --git a/.github/workflows/fuzz_merkledb.yml b/.github/workflows/fuzz_merkledb.yml index 09c58ffd38e0..7659f07a7c14 100644 --- a/.github/workflows/fuzz_merkledb.yml +++ b/.github/workflows/fuzz_merkledb.yml @@ -20,7 +20,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '~1.21.7' + go-version: '~1.21.8' check-latest: true - name: Run merkledb fuzz tests shell: bash diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3e509ed11064..444065b67182 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ To start developing on AvalancheGo, you'll need a few things installed. -- Golang version >= 1.21.7 +- Golang version >= 1.21.8 - gcc - g++ diff --git a/Dockerfile b/Dockerfile index 62594f64c766..035b203d8ca2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ # README.md # go.mod # ============= Compilation Stage ================ -FROM golang:1.21.7-bullseye AS builder +FROM golang:1.21.8-bullseye AS builder WORKDIR /build # Copy and download avalanche dependencies using go mod diff --git a/README.md b/README.md index 7eec5b925b80..b4e213d368b3 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ The minimum recommended hardware specification for nodes connected to Mainnet is If you plan to build AvalancheGo from source, you will also need the following software: -- [Go](https://golang.org/doc/install) version >= 1.21.7 +- [Go](https://golang.org/doc/install) version >= 1.21.8 - [gcc](https://gcc.gnu.org/) - g++ diff --git a/proto/Dockerfile.buf b/proto/Dockerfile.buf index 3007f58667b3..2b826b17062f 100644 --- a/proto/Dockerfile.buf +++ b/proto/Dockerfile.buf @@ -6,7 +6,7 @@ RUN apt-get update && apt -y install bash curl unzip git WORKDIR /opt RUN \ - curl -L https://go.dev/dl/go1.21.7.linux-amd64.tar.gz > golang.tar.gz && \ + curl -L https://go.dev/dl/go1.21.8.linux-amd64.tar.gz > golang.tar.gz && \ mkdir golang && \ tar -zxvf golang.tar.gz -C golang/ diff --git a/scripts/build_avalanche.sh b/scripts/build_avalanche.sh index eda63a5b959b..a2b06508668e 100755 --- a/scripts/build_avalanche.sh +++ b/scripts/build_avalanche.sh @@ -27,7 +27,7 @@ done # Dockerfile # README.md # go.mod -go_version_minimum="1.21.7" +go_version_minimum="1.21.8" go_version() { go version | sed -nE -e 's/[^0-9.]+([0-9.]+).+/\1/p' From 66ae8ef310411aaa67cf98ad96c0d4b085e1cce0 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 6 Mar 2024 10:51:17 -0500 Subject: [PATCH 06/11] Cleanup consensus metrics (#2815) --- snow/engine/snowman/issuer.go | 5 --- snow/engine/snowman/transitive.go | 72 ++++++++++++------------------- snow/engine/snowman/voter.go | 2 +- 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/snow/engine/snowman/issuer.go b/snow/engine/snowman/issuer.go index d952dfe2cc6b..b3677d3cc21e 100644 --- a/snow/engine/snowman/issuer.go +++ b/snow/engine/snowman/issuer.go @@ -41,11 +41,6 @@ func (i *issuer) Abandon(ctx context.Context, _ ids.ID) { i.t.removeFromPending(i.blk) i.t.addToNonVerifieds(i.blk) i.t.blocked.Abandon(ctx, blkID) - - // Tracks performance statistics - i.t.metrics.numRequests.Set(float64(i.t.blkReqs.Len())) - i.t.metrics.numBlocked.Set(float64(len(i.t.pending))) - i.t.metrics.numBlockers.Set(float64(i.t.blocked.Len())) } i.abandoned = true } diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index b446401a88c7..a1335a1ad7fd 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -210,7 +210,7 @@ func (t *Transitive) Gossip(ctx context.Context) error { return nil } - lastAccepted, err := t.GetBlock(ctx, lastAcceptedID) + lastAccepted, err := t.getBlock(ctx, lastAcceptedID) if err != nil { t.Ctx.Log.Warn("dropping gossip request", zap.String("reason", "block couldn't be loaded"), @@ -296,7 +296,7 @@ func (t *Transitive) Put(ctx context.Context, nodeID ids.NodeID, requestID uint3 if _, err := t.issueFrom(ctx, nodeID, blk, issuedMetric); err != nil { return err } - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (t *Transitive) GetFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32) error { @@ -319,9 +319,7 @@ func (t *Transitive) GetFailed(ctx context.Context, nodeID ids.NodeID, requestID // Because the get request was dropped, we no longer expect blkID to be issued. t.blocked.Abandon(ctx, blkID) - t.metrics.numRequests.Set(float64(t.blkReqs.Len())) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, blkID ids.ID, requestedHeight uint64) error { @@ -335,7 +333,7 @@ func (t *Transitive) PullQuery(ctx context.Context, nodeID ids.NodeID, requestID return err } - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (t *Transitive) PushQuery(ctx context.Context, nodeID ids.NodeID, requestID uint32, blkBytes []byte, requestedHeight uint64) error { @@ -376,7 +374,7 @@ func (t *Transitive) PushQuery(ctx context.Context, nodeID ids.NodeID, requestID return err } - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (t *Transitive) Chits(ctx context.Context, nodeID ids.NodeID, requestID uint32, preferredID ids.ID, preferredIDAtHeight ids.ID, acceptedID ids.ID) error { @@ -432,8 +430,7 @@ func (t *Transitive) Chits(ctx context.Context, nodeID ids.NodeID, requestID uin } t.blocked.Register(ctx, v) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (t *Transitive) QueryFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32) error { @@ -450,8 +447,7 @@ func (t *Transitive) QueryFailed(ctx context.Context, nodeID ids.NodeID, request requestID: requestID, }, ) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) } func (*Transitive) Timeout(context.Context) error { @@ -474,7 +470,7 @@ func (t *Transitive) Notify(ctx context.Context, msg common.Message) error { case common.PendingTxs: // the pending txs message means we should attempt to build a block. t.pendingBuildBlocks++ - return t.buildBlocks(ctx) + return t.executeDeferredWork(ctx) case common.StateSyncDone: t.Ctx.StateSyncing.Set(false) return nil @@ -497,7 +493,7 @@ func (t *Transitive) Start(ctx context.Context, startReqID uint32) error { return err } - lastAccepted, err := t.GetBlock(ctx, lastAcceptedID) + lastAccepted, err := t.getBlock(ctx, lastAcceptedID) if err != nil { t.Ctx.Log.Error("failed to get last accepted block", zap.Error(err), @@ -549,7 +545,7 @@ func (t *Transitive) Start(ctx context.Context, startReqID uint32) error { return fmt.Errorf("failed to notify VM that consensus is starting: %w", err) } - return nil + return t.executeDeferredWork(ctx) } func (t *Transitive) HealthCheck(ctx context.Context) (interface{}, error) { @@ -580,7 +576,19 @@ func (t *Transitive) HealthCheck(ctx context.Context) (interface{}, error) { return intf, fmt.Errorf("vm: %w ; consensus: %w", vmErr, consensusErr) } -func (t *Transitive) GetBlock(ctx context.Context, blkID ids.ID) (snowman.Block, error) { +func (t *Transitive) executeDeferredWork(ctx context.Context) error { + if err := t.buildBlocks(ctx); err != nil { + return err + } + + t.metrics.numRequests.Set(float64(t.blkReqs.Len())) + t.metrics.numBlocked.Set(float64(len(t.pending))) + t.metrics.numBlockers.Set(float64(t.blocked.Len())) + t.metrics.numNonVerifieds.Set(float64(t.nonVerifieds.Len())) + return nil +} + +func (t *Transitive) getBlock(ctx context.Context, blkID ids.ID) (snowman.Block, error) { if blk, ok := t.pending[blkID]; ok { return blk, nil } @@ -733,7 +741,7 @@ func (t *Transitive) issueFromByID( blkID ids.ID, issuedMetric prometheus.Counter, ) (bool, error) { - blk, err := t.GetBlock(ctx, blkID) + blk, err := t.getBlock(ctx, blkID) if err != nil { t.sendRequest(ctx, nodeID, blkID, issuedMetric) return false, nil @@ -759,7 +767,7 @@ func (t *Transitive) issueFrom( blkID = blk.Parent() var err error - blk, err = t.GetBlock(ctx, blkID) + blk, err = t.getBlock(ctx, blkID) // If we don't have this ancestor, request it from [vdr] if err != nil || !blk.Status().Fetched() { @@ -780,10 +788,6 @@ func (t *Transitive) issueFrom( // dependencies may still be waiting. Therefore, they should abandoned. t.blocked.Abandon(ctx, blkID) } - - // Tracks performance statistics - t.metrics.numRequests.Set(float64(t.blkReqs.Len())) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return issued, t.errs.Err } @@ -804,7 +808,7 @@ func (t *Transitive) issueWithAncestors( return false, err } blkID = blk.Parent() - blk, err = t.GetBlock(ctx, blkID) + blk, err = t.getBlock(ctx, blkID) if err != nil { status = choices.Unknown break @@ -826,7 +830,6 @@ func (t *Transitive) issueWithAncestors( // We don't have this block and have no reason to expect that we will get it. // Abandon the block to avoid a memory leak. t.blocked.Abandon(ctx, blkID) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return false, t.errs.Err } @@ -869,7 +872,7 @@ func (t *Transitive) issue( // block on the parent if needed parentID := blk.Parent() - if parent, err := t.GetBlock(ctx, parentID); err != nil || !(t.Consensus.Decided(parent) || t.Consensus.Processing(parentID)) { + if parent, err := t.getBlock(ctx, parentID); err != nil || !(t.Consensus.Decided(parent) || t.Consensus.Processing(parentID)) { t.Ctx.Log.Verbo("block waiting for parent to be issued", zap.Stringer("blkID", blkID), zap.Stringer("parentID", parentID), @@ -878,11 +881,6 @@ func (t *Transitive) issue( } t.blocked.Register(ctx, i) - - // Tracks performance statistics - t.metrics.numRequests.Set(float64(t.blkReqs.Len())) - t.metrics.numBlocked.Set(float64(len(t.pending))) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return t.errs.Err } @@ -912,9 +910,6 @@ func (t *Transitive) sendRequest( zap.Stringer("blkID", blkID), ) t.Sender.SendGet(ctx, nodeID, t.requestID, blkID) - - // Tracks performance statistics - t.metrics.numRequests.Set(float64(t.blkReqs.Len())) } // Send a query for this block. If push is set to true, blkBytes will be used to @@ -990,7 +985,7 @@ func (t *Transitive) deliver( // longer pending t.removeFromPending(blk) parentID := blk.Parent() - parent, err := t.GetBlock(ctx, parentID) + parent, err := t.getBlock(ctx, parentID) // Because the dependency must have been fulfilled by the time this function // is called - we don't expect [err] to be non-nil. But it is handled for // completness and future proofing. @@ -998,8 +993,6 @@ func (t *Transitive) deliver( // if the parent isn't processing or the last accepted block, then this // block is effectively rejected t.blocked.Abandon(ctx, blkID) - t.metrics.numBlocked.Set(float64(len(t.pending))) // Tracks performance statistics - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return t.errs.Err } @@ -1012,8 +1005,6 @@ func (t *Transitive) deliver( } if !blkAdded { t.blocked.Abandon(ctx, blkID) - t.metrics.numBlocked.Set(float64(len(t.pending))) // Tracks performance statistics - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return t.errs.Err } @@ -1077,11 +1068,6 @@ func (t *Transitive) deliver( // If we should issue multiple queries at the same time, we need to repoll t.repoll(ctx) - - // Tracks performance statistics - t.metrics.numRequests.Set(float64(t.blkReqs.Len())) - t.metrics.numBlocked.Set(float64(len(t.pending))) - t.metrics.numBlockers.Set(float64(t.blocked.Len())) return t.errs.Err } @@ -1108,7 +1094,6 @@ func (t *Transitive) addToNonVerifieds(blk snowman.Block) { if t.nonVerifieds.Has(parentID) || t.Consensus.Processing(parentID) { t.nonVerifieds.Add(blkID, parentID) t.nonVerifiedCache.Put(blkID, blk) - t.metrics.numNonVerifieds.Set(float64(t.nonVerifieds.Len())) } } @@ -1140,7 +1125,6 @@ func (t *Transitive) addUnverifiedBlockToConsensus( issuedMetric.Inc() t.nonVerifieds.Remove(blkID) t.nonVerifiedCache.Evict(blkID) - t.metrics.numNonVerifieds.Set(float64(t.nonVerifieds.Len())) t.metrics.issuerStake.Observe(float64(t.Validators.GetWeight(t.Ctx.SubnetID, nodeID))) t.Ctx.Log.Verbo("adding block to consensus", zap.Stringer("nodeID", nodeID), diff --git a/snow/engine/snowman/voter.go b/snow/engine/snowman/voter.go index 0a029e870ec2..94ddaa2b6bfc 100644 --- a/snow/engine/snowman/voter.go +++ b/snow/engine/snowman/voter.go @@ -107,7 +107,7 @@ func (v *voter) getProcessingAncestor(ctx context.Context, initialVote ids.ID) ( // have at our disposal as a best-effort mechanism to find a valid ancestor. bubbledVote := v.t.nonVerifieds.GetAncestor(initialVote) for { - blk, err := v.t.GetBlock(ctx, bubbledVote) + blk, err := v.t.getBlock(ctx, bubbledVote) // If we cannot retrieve the block, drop [vote] if err != nil { v.t.Ctx.Log.Debug("dropping vote", From 5793120edb33190b997068d0a7f99d1dddc66e55 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 6 Mar 2024 10:57:34 -0500 Subject: [PATCH 07/11] Remove peerlist push gossip (#2791) --- config/config.go | 18 +++---------- config/flags.go | 4 --- config/keys.go | 4 --- network/config.go | 16 ------------ network/network.go | 19 -------------- network/network_test.go | 10 +++----- network/peer/peer.go | 43 -------------------------------- network/test_network.go | 10 +++----- tests/fixture/tmpnet/defaults.go | 22 ++++++++-------- 9 files changed, 20 insertions(+), 126 deletions(-) diff --git a/config/config.go b/config/config.go index b9dc285daec3..f0d8d643677b 100644 --- a/config/config.go +++ b/config/config.go @@ -55,7 +55,6 @@ const ( keystoreDeprecationMsg = "keystore API is deprecated" acceptedFrontierGossipDeprecationMsg = "push-based accepted frontier gossip is deprecated" - peerListPushGossipDeprecationMsg = "push-based peer list gossip is deprecated" ) var ( @@ -72,11 +71,6 @@ var ( ConsensusGossipOnAcceptNonValidatorSizeKey: acceptedFrontierGossipDeprecationMsg, ConsensusGossipOnAcceptPeerSizeKey: acceptedFrontierGossipDeprecationMsg, - NetworkPeerListValidatorGossipSizeKey: peerListPushGossipDeprecationMsg, - NetworkPeerListNonValidatorGossipSizeKey: peerListPushGossipDeprecationMsg, - NetworkPeerListPeersGossipSizeKey: peerListPushGossipDeprecationMsg, - NetworkPeerListGossipFreqKey: peerListPushGossipDeprecationMsg, - SnowRogueCommitThresholdKey: commitThresholdDeprecationMsg, SnowVirtuousCommitThresholdKey: commitThresholdDeprecationMsg, } @@ -386,13 +380,9 @@ func getNetworkConfig( }, PeerListGossipConfig: network.PeerListGossipConfig{ - PeerListNumValidatorIPs: v.GetUint32(NetworkPeerListNumValidatorIPsKey), - PeerListValidatorGossipSize: v.GetUint32(NetworkPeerListValidatorGossipSizeKey), - PeerListNonValidatorGossipSize: v.GetUint32(NetworkPeerListNonValidatorGossipSizeKey), - PeerListPeersGossipSize: v.GetUint32(NetworkPeerListPeersGossipSizeKey), - PeerListGossipFreq: v.GetDuration(NetworkPeerListGossipFreqKey), - PeerListPullGossipFreq: v.GetDuration(NetworkPeerListPullGossipFreqKey), - PeerListBloomResetFreq: v.GetDuration(NetworkPeerListBloomResetFreqKey), + PeerListNumValidatorIPs: v.GetUint32(NetworkPeerListNumValidatorIPsKey), + PeerListPullGossipFreq: v.GetDuration(NetworkPeerListPullGossipFreqKey), + PeerListBloomResetFreq: v.GetDuration(NetworkPeerListBloomResetFreqKey), }, DelayConfig: network.DelayConfig{ @@ -426,8 +416,6 @@ func getNetworkConfig( return network.Config{}, fmt.Errorf("%s must be in [0,1]", NetworkHealthMaxPortionSendQueueFillKey) case config.DialerConfig.ConnectionTimeout < 0: return network.Config{}, fmt.Errorf("%q must be >= 0", NetworkOutboundConnectionTimeoutKey) - case config.PeerListGossipFreq < 0: - return network.Config{}, fmt.Errorf("%s must be >= 0", NetworkPeerListGossipFreqKey) case config.PeerListPullGossipFreq < 0: return network.Config{}, fmt.Errorf("%s must be >= 0", NetworkPeerListPullGossipFreqKey) case config.PeerListBloomResetFreq < 0: diff --git a/config/flags.go b/config/flags.go index 0d366dbfbd3c..39d025dcd6bd 100644 --- a/config/flags.go +++ b/config/flags.go @@ -128,10 +128,6 @@ func addNodeFlags(fs *pflag.FlagSet) { // Peer List Gossip fs.Uint(NetworkPeerListNumValidatorIPsKey, constants.DefaultNetworkPeerListNumValidatorIPs, "Number of validator IPs to gossip to other nodes") - fs.Uint(NetworkPeerListValidatorGossipSizeKey, constants.DefaultNetworkPeerListValidatorGossipSize, "Number of validators that the node will gossip peer list to") - fs.Uint(NetworkPeerListNonValidatorGossipSizeKey, constants.DefaultNetworkPeerListNonValidatorGossipSize, "Number of non-validators that the node will gossip peer list to") - fs.Uint(NetworkPeerListPeersGossipSizeKey, constants.DefaultNetworkPeerListPeersGossipSize, "Number of total peers (including non-validators and validators) that the node will gossip peer list to") - fs.Duration(NetworkPeerListGossipFreqKey, constants.DefaultNetworkPeerListGossipFreq, "Frequency to gossip peers to other nodes") fs.Duration(NetworkPeerListPullGossipFreqKey, constants.DefaultNetworkPeerListPullGossipFreq, "Frequency to request peers from other nodes") fs.Duration(NetworkPeerListBloomResetFreqKey, constants.DefaultNetworkPeerListBloomResetFreq, "Frequency to recalculate the bloom filter used to request new peers from other nodes") diff --git a/config/keys.go b/config/keys.go index 6fc29ba007ab..195be55434ce 100644 --- a/config/keys.go +++ b/config/keys.go @@ -87,10 +87,6 @@ const ( NetworkHealthMaxSendFailRateKey = "network-health-max-send-fail-rate" NetworkHealthMaxOutstandingDurationKey = "network-health-max-outstanding-request-duration" NetworkPeerListNumValidatorIPsKey = "network-peer-list-num-validator-ips" - NetworkPeerListValidatorGossipSizeKey = "network-peer-list-validator-gossip-size" - NetworkPeerListNonValidatorGossipSizeKey = "network-peer-list-non-validator-gossip-size" - NetworkPeerListPeersGossipSizeKey = "network-peer-list-peers-gossip-size" - NetworkPeerListGossipFreqKey = "network-peer-list-gossip-frequency" NetworkPeerListPullGossipFreqKey = "network-peer-list-pull-gossip-frequency" NetworkPeerListBloomResetFreqKey = "network-peer-list-bloom-reset-frequency" NetworkInitialReconnectDelayKey = "network-initial-reconnect-delay" diff --git a/network/config.go b/network/config.go index 64c57d120cd6..ed82ea507e8e 100644 --- a/network/config.go +++ b/network/config.go @@ -58,22 +58,6 @@ type PeerListGossipConfig struct { // gossip event. PeerListNumValidatorIPs uint32 `json:"peerListNumValidatorIPs"` - // PeerListValidatorGossipSize is the number of validators to gossip the IPs - // to in every IP gossip event. - PeerListValidatorGossipSize uint32 `json:"peerListValidatorGossipSize"` - - // PeerListNonValidatorGossipSize is the number of non-validators to gossip - // the IPs to in every IP gossip event. - PeerListNonValidatorGossipSize uint32 `json:"peerListNonValidatorGossipSize"` - - // PeerListPeersGossipSize is the number of peers to gossip - // the IPs to in every IP gossip event. - PeerListPeersGossipSize uint32 `json:"peerListPeersGossipSize"` - - // PeerListGossipFreq is the frequency that this node will attempt to gossip - // signed IPs to its peers. - PeerListGossipFreq time.Duration `json:"peerListGossipFreq"` - // PeerListPullGossipFreq is the frequency that this node will attempt to // request signed IPs from its peers. PeerListPullGossipFreq time.Duration `json:"peerListPullGossipFreq"` diff --git a/network/network.go b/network/network.go index 5e4b3cdc4e82..c487d8244b68 100644 --- a/network/network.go +++ b/network/network.go @@ -1157,12 +1157,10 @@ func (n *network) NodeUptime(subnetID ids.ID) (UptimeResult, error) { } func (n *network) runTimers() { - pushGossipPeerlists := time.NewTicker(n.config.PeerListGossipFreq) pullGossipPeerlists := time.NewTicker(n.config.PeerListPullGossipFreq) resetPeerListBloom := time.NewTicker(n.config.PeerListBloomResetFreq) updateUptimes := time.NewTicker(n.config.UptimeMetricFreq) defer func() { - pushGossipPeerlists.Stop() resetPeerListBloom.Stop() updateUptimes.Stop() }() @@ -1171,8 +1169,6 @@ func (n *network) runTimers() { select { case <-n.onCloseCtx.Done(): return - case <-pushGossipPeerlists.C: - n.pushGossipPeerLists() case <-pullGossipPeerlists.C: n.pullGossipPeerLists() case <-resetPeerListBloom.C: @@ -1209,21 +1205,6 @@ func (n *network) runTimers() { } } -// pushGossipPeerLists gossips validators to peers in the network -func (n *network) pushGossipPeerLists() { - peers := n.samplePeers( - constants.PrimaryNetworkID, - int(n.config.PeerListValidatorGossipSize), - int(n.config.PeerListNonValidatorGossipSize), - int(n.config.PeerListPeersGossipSize), - subnets.NoOpAllower, - ) - - for _, p := range peers { - p.StartSendPeerList() - } -} - // pullGossipPeerLists requests validators from peers in the network func (n *network) pullGossipPeerLists() { peers := n.samplePeers( diff --git a/network/network_test.go b/network/network_test.go index 0f95d722b654..f48b904fa1b9 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -49,13 +49,9 @@ var ( SendFailRateHalflife: time.Second, } defaultPeerListGossipConfig = PeerListGossipConfig{ - PeerListNumValidatorIPs: 100, - PeerListValidatorGossipSize: 100, - PeerListNonValidatorGossipSize: 100, - PeerListPeersGossipSize: 100, - PeerListGossipFreq: time.Second, - PeerListPullGossipFreq: time.Second, - PeerListBloomResetFreq: constants.DefaultNetworkPeerListBloomResetFreq, + PeerListNumValidatorIPs: 100, + PeerListPullGossipFreq: time.Second, + PeerListBloomResetFreq: constants.DefaultNetworkPeerListBloomResetFreq, } defaultTimeoutConfig = TimeoutConfig{ PingPongTimeout: 30 * time.Second, diff --git a/network/peer/peer.go b/network/peer/peer.go index 58b4e648125e..c747c1d816b7 100644 --- a/network/peer/peer.go +++ b/network/peer/peer.go @@ -92,11 +92,6 @@ type Peer interface { // guaranteed not to be delivered to the peer. Send(ctx context.Context, msg message.OutboundMessage) bool - // StartSendPeerList attempts to send a PeerList message to this peer on - // this peer's gossip routine. It is not guaranteed that a PeerList will be - // sent. - StartSendPeerList() - // StartSendGetPeerList attempts to send a GetPeerList message to this peer // on this peer's gossip routine. It is not guaranteed that a GetPeerList // will be sent. @@ -186,10 +181,6 @@ type peer struct { // Must only be accessed atomically lastSent, lastReceived int64 - // peerListChan signals that we should attempt to send a PeerList to this - // peer - peerListChan chan struct{} - // getPeerListChan signals that we should attempt to send a GetPeerList to // this peer getPeerListChan chan struct{} @@ -219,7 +210,6 @@ func Start( onClosingCtxCancel: onClosingCtxCancel, onClosed: make(chan struct{}), observedUptimes: make(map[ids.ID]uint32), - peerListChan: make(chan struct{}, 1), getPeerListChan: make(chan struct{}, 1), } @@ -327,13 +317,6 @@ func (p *peer) Send(ctx context.Context, msg message.OutboundMessage) bool { return p.messageQueue.Push(ctx, msg) } -func (p *peer) StartSendPeerList() { - select { - case p.peerListChan <- struct{}{}: - default: - } -} - func (p *peer) StartSendGetPeerList() { select { case p.getPeerListChan <- struct{}{}: @@ -656,32 +639,6 @@ func (p *peer) sendNetworkMessages() { for { select { - case <-p.peerListChan: - peerIPs := p.Config.Network.Peers(p.id, bloom.EmptyFilter, nil) - if len(peerIPs) == 0 { - p.Log.Verbo( - "skipping peer gossip as there are no unknown peers", - zap.Stringer("nodeID", p.id), - ) - continue - } - - // Bypass throttling is disabled here to follow the non-handshake - // message sending pattern. - msg, err := p.Config.MessageCreator.PeerList(peerIPs, false /*=bypassThrottling*/) - if err != nil { - p.Log.Error("failed to create peer list message", - zap.Stringer("nodeID", p.id), - zap.Error(err), - ) - continue - } - - if !p.Send(p.onClosingCtx, msg) { - p.Log.Debug("failed to send peer list", - zap.Stringer("nodeID", p.id), - ) - } case <-p.getPeerListChan: knownPeersFilter, knownPeersSalt := p.Config.Network.KnownPeers() msg, err := p.Config.MessageCreator.GetPeerList(knownPeersFilter, knownPeersSalt) diff --git a/network/test_network.go b/network/test_network.go index 1cb56127c1a7..25039ad046ba 100644 --- a/network/test_network.go +++ b/network/test_network.go @@ -152,13 +152,9 @@ func NewTestNetwork( }, PeerListGossipConfig: PeerListGossipConfig{ - PeerListNumValidatorIPs: constants.DefaultNetworkPeerListNumValidatorIPs, - PeerListValidatorGossipSize: constants.DefaultNetworkPeerListValidatorGossipSize, - PeerListNonValidatorGossipSize: constants.DefaultNetworkPeerListNonValidatorGossipSize, - PeerListPeersGossipSize: constants.DefaultNetworkPeerListPeersGossipSize, - PeerListGossipFreq: constants.DefaultNetworkPeerListGossipFreq, - PeerListPullGossipFreq: constants.DefaultNetworkPeerListPullGossipFreq, - PeerListBloomResetFreq: constants.DefaultNetworkPeerListBloomResetFreq, + PeerListNumValidatorIPs: constants.DefaultNetworkPeerListNumValidatorIPs, + PeerListPullGossipFreq: constants.DefaultNetworkPeerListPullGossipFreq, + PeerListBloomResetFreq: constants.DefaultNetworkPeerListBloomResetFreq, }, DelayConfig: DelayConfig{ diff --git a/tests/fixture/tmpnet/defaults.go b/tests/fixture/tmpnet/defaults.go index bc826ad62345..aac7a1fee912 100644 --- a/tests/fixture/tmpnet/defaults.go +++ b/tests/fixture/tmpnet/defaults.go @@ -38,17 +38,17 @@ const ( func DefaultFlags() FlagsMap { // Supply only non-default configuration to ensure that default values will be used. return FlagsMap{ - config.NetworkPeerListGossipFreqKey: "250ms", - config.NetworkMaxReconnectDelayKey: "1s", - config.PublicIPKey: "127.0.0.1", - config.HTTPHostKey: "127.0.0.1", - config.StakingHostKey: "127.0.0.1", - config.HealthCheckFreqKey: "2s", - config.AdminAPIEnabledKey: true, - config.IndexEnabledKey: true, - config.LogDisplayLevelKey: "INFO", - config.LogLevelKey: "DEBUG", - config.MinStakeDurationKey: DefaultMinStakeDuration.String(), + config.NetworkPeerListPullGossipFreqKey: "250ms", + config.NetworkMaxReconnectDelayKey: "1s", + config.PublicIPKey: "127.0.0.1", + config.HTTPHostKey: "127.0.0.1", + config.StakingHostKey: "127.0.0.1", + config.HealthCheckFreqKey: "2s", + config.AdminAPIEnabledKey: true, + config.IndexEnabledKey: true, + config.LogDisplayLevelKey: "INFO", + config.LogLevelKey: "DEBUG", + config.MinStakeDurationKey: DefaultMinStakeDuration.String(), } } From dc2c5d0ba789420f776f4f0e8b953359d03a0f3f Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 6 Mar 2024 11:17:20 -0500 Subject: [PATCH 08/11] Remove bitmaskCodec (#2792) --- network/peer/msg_length.go | 32 +++----------- network/peer/msg_length_test.go | 76 ++++----------------------------- 2 files changed, 14 insertions(+), 94 deletions(-) diff --git a/network/peer/msg_length.go b/network/peer/msg_length.go index 625034913d9f..0072734c5875 100644 --- a/network/peer/msg_length.go +++ b/network/peer/msg_length.go @@ -12,29 +12,20 @@ import ( ) var ( - errInvalidMaxMessageLength = errors.New("invalid maximum message length") errInvalidMessageLength = errors.New("invalid message length") errMaxMessageLengthExceeded = errors.New("maximum message length exceeded") ) -// Used to mask the most significant bit that was used to indicate that the -// message format uses protocol buffers. -// -// TODO: Once the v1.11 is activated, this mask should be removed. -const bitmaskCodec = uint32(1 << 31) - // Assumes the specified [msgLen] will never >= 1<<31. func writeMsgLen(msgLen uint32, maxMsgLen uint32) ([wrappers.IntLen]byte, error) { - if maxMsgLen >= bitmaskCodec { + if msgLen > maxMsgLen { return [wrappers.IntLen]byte{}, fmt.Errorf( - "%w; maximum message length must be <%d to be able to embed codec information at most significant bit", - errInvalidMaxMessageLength, - bitmaskCodec, + "%w; the message length %d exceeds the specified limit %d", + errMaxMessageLengthExceeded, + msgLen, + maxMsgLen, ) } - if msgLen > maxMsgLen { - return [wrappers.IntLen]byte{}, fmt.Errorf("%w; the message length %d exceeds the specified limit %d", errMaxMessageLengthExceeded, msgLen, maxMsgLen) - } b := [wrappers.IntLen]byte{} binary.BigEndian.PutUint32(b[:], msgLen) @@ -44,13 +35,6 @@ func writeMsgLen(msgLen uint32, maxMsgLen uint32) ([wrappers.IntLen]byte, error) // Assumes the read [msgLen] will never >= 1<<31. func readMsgLen(b []byte, maxMsgLen uint32) (uint32, error) { - if maxMsgLen >= bitmaskCodec { - return 0, fmt.Errorf( - "%w; maximum message length must be <%d to be able to embed codec information at most significant bit", - errInvalidMaxMessageLength, - bitmaskCodec, - ) - } if len(b) != wrappers.IntLen { return 0, fmt.Errorf( "%w; readMsgLen only supports 4 bytes (got %d bytes)", @@ -61,12 +45,6 @@ func readMsgLen(b []byte, maxMsgLen uint32) (uint32, error) { // parse the message length msgLen := binary.BigEndian.Uint32(b) - - // Because we always use proto messages, there's no need to check the most - // significant bit to inspect the message format. So, we just zero the proto - // flag. - msgLen &^= bitmaskCodec - if msgLen > maxMsgLen { return 0, fmt.Errorf( "%w; the message length %d exceeds the specified limit %d", diff --git a/network/peer/msg_length_test.go b/network/peer/msg_length_test.go index 97866a7d95cf..c7a587638148 100644 --- a/network/peer/msg_length_test.go +++ b/network/peer/msg_length_test.go @@ -21,19 +21,9 @@ func TestWriteMsgLen(t *testing.T) { expectedErr error }{ { - msgLen: math.MaxUint32, - msgLimit: math.MaxUint32, - expectedErr: errInvalidMaxMessageLength, - }, - { - msgLen: bitmaskCodec, - msgLimit: bitmaskCodec, - expectedErr: errInvalidMaxMessageLength, - }, - { - msgLen: bitmaskCodec - 1, - msgLimit: bitmaskCodec - 1, - expectedErr: nil, + msgLen: constants.DefaultMaxMessageSize, + msgLimit: 1, + expectedErr: errMaxMessageLengthExceeded, }, { msgLen: constants.DefaultMaxMessageSize, @@ -45,11 +35,6 @@ func TestWriteMsgLen(t *testing.T) { msgLimit: constants.DefaultMaxMessageSize, expectedErr: nil, }, - { - msgLen: constants.DefaultMaxMessageSize, - msgLimit: 1, - expectedErr: errMaxMessageLengthExceeded, - }, } for _, tv := range tt { msgLenBytes, err := writeMsgLen(tv.msgLen, tv.msgLimit) @@ -73,12 +58,6 @@ func TestReadMsgLen(t *testing.T) { expectedErr error expectedMsgLen uint32 }{ - { - msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF}, - msgLimit: math.MaxUint32, - expectedErr: errInvalidMaxMessageLength, - expectedMsgLen: 0, - }, { msgLenBytes: []byte{0b11111111, 0xFF}, msgLimit: math.MaxInt32, @@ -86,26 +65,20 @@ func TestReadMsgLen(t *testing.T) { expectedMsgLen: 0, }, { - msgLenBytes: []byte{0b11111111, 0xFF, 0xFF, 0xFF}, + msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF}, msgLimit: constants.DefaultMaxMessageSize, expectedErr: errMaxMessageLengthExceeded, expectedMsgLen: 0, }, { - msgLenBytes: []byte{0b11111111, 0xFF, 0xFF, 0xFF}, - msgLimit: math.MaxInt32, - expectedErr: nil, - expectedMsgLen: math.MaxInt32, - }, - { - msgLenBytes: []byte{0b10000000, 0x00, 0x00, 0x01}, - msgLimit: math.MaxInt32, + msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF}, + msgLimit: math.MaxUint32, expectedErr: nil, - expectedMsgLen: 1, + expectedMsgLen: math.MaxUint32, }, { - msgLenBytes: []byte{0b10000000, 0x00, 0x00, 0x01}, - msgLimit: 1, + msgLenBytes: []byte{0x00, 0x00, 0x00, 0x01}, + msgLimit: 10, expectedErr: nil, expectedMsgLen: 1, }, @@ -126,34 +99,3 @@ func TestReadMsgLen(t *testing.T) { require.Equal(tv.expectedMsgLen, msgLenAfterWrite) } } - -func TestBackwardsCompatibleReadMsgLen(t *testing.T) { - require := require.New(t) - - tt := []struct { - msgLenBytes []byte - msgLimit uint32 - expectedMsgLen uint32 - }{ - { - msgLenBytes: []byte{0b01111111, 0xFF, 0xFF, 0xFF}, - msgLimit: math.MaxInt32, - expectedMsgLen: math.MaxInt32, - }, - { - msgLenBytes: []byte{0b00000000, 0x00, 0x00, 0x01}, - msgLimit: math.MaxInt32, - expectedMsgLen: 1, - }, - { - msgLenBytes: []byte{0b00000000, 0x00, 0x00, 0x01}, - msgLimit: 1, - expectedMsgLen: 1, - }, - } - for _, tv := range tt { - msgLen, err := readMsgLen(tv.msgLenBytes, tv.msgLimit) - require.NoError(err) - require.Equal(tv.expectedMsgLen, msgLen) - } -} From dc0362266e72a9cb09a66f054a2a1676a008c615 Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Wed, 6 Mar 2024 11:47:43 -0500 Subject: [PATCH 09/11] Use `BaseTx` in P-chain wallet (#2731) Co-authored-by: Alberto Benegiamo Co-authored-by: Stephen Buttolph --- wallet/chain/p/builder.go | 27 +++++++++++---------------- wallet/chain/p/builder_test.go | 2 +- wallet/chain/p/wallet.go | 2 -- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/wallet/chain/p/builder.go b/wallet/chain/p/builder.go index 85f9b6111010..fff2b5860201 100644 --- a/wallet/chain/p/builder.go +++ b/wallet/chain/p/builder.go @@ -51,16 +51,14 @@ type Builder interface { options ...common.Option, ) (map[ids.ID]uint64, error) - // NewBaseTx creates a new simple value transfer. Because the P-chain - // doesn't intend for balance transfers to occur, this method is expensive - // and abuses the creation of subnets. + // NewBaseTx creates a new simple value transfer. // // - [outputs] specifies all the recipients and amounts that should be sent // from this transaction. NewBaseTx( outputs []*avax.TransferableOutput, options ...common.Option, - ) (*txs.CreateSubnetTx, error) + ) (*txs.BaseTx, error) // NewAddValidatorTx creates a new validator of the primary network. // @@ -300,9 +298,9 @@ func (b *builder) GetImportableBalance( func (b *builder) NewBaseTx( outputs []*avax.TransferableOutput, options ...common.Option, -) (*txs.CreateSubnetTx, error) { +) (*txs.BaseTx, error) { toBurn := map[ids.ID]uint64{ - b.backend.AVAXAssetID(): b.backend.CreateSubnetTxFee(), + b.backend.AVAXAssetID(): b.backend.BaseTxFee(), } for _, out := range outputs { assetID := out.AssetID() @@ -322,16 +320,13 @@ func (b *builder) NewBaseTx( outputs = append(outputs, changeOutputs...) avax.SortTransferableOutputs(outputs, txs.Codec) // sort the outputs - tx := &txs.CreateSubnetTx{ - BaseTx: txs.BaseTx{BaseTx: avax.BaseTx{ - NetworkID: b.backend.NetworkID(), - BlockchainID: constants.PlatformChainID, - Ins: inputs, - Outs: outputs, - Memo: ops.Memo(), - }}, - Owner: &secp256k1fx.OutputOwners{}, - } + tx := &txs.BaseTx{BaseTx: avax.BaseTx{ + NetworkID: b.backend.NetworkID(), + BlockchainID: constants.PlatformChainID, + Ins: inputs, + Outs: outputs, + Memo: ops.Memo(), + }} return tx, b.initCtx(tx) } diff --git a/wallet/chain/p/builder_test.go b/wallet/chain/p/builder_test.go index c971b0200b02..409c524ba938 100644 --- a/wallet/chain/p/builder_test.go +++ b/wallet/chain/p/builder_test.go @@ -87,7 +87,7 @@ func TestBaseTx(t *testing.T) { require.Len(ins, 2) require.Len(outs, 2) - expectedConsumed := testCtx.CreateSubnetTxFee() + outputsToMove[0].Out.Amount() + expectedConsumed := testCtx.BaseTxFee() + outputsToMove[0].Out.Amount() consumed := ins[0].In.Amount() + ins[1].In.Amount() - outs[0].Out.Amount() require.Equal(expectedConsumed, consumed) require.Equal(outputsToMove[0], outs[1]) diff --git a/wallet/chain/p/wallet.go b/wallet/chain/p/wallet.go index 44cc7e2a4da4..61655e0d157f 100644 --- a/wallet/chain/p/wallet.go +++ b/wallet/chain/p/wallet.go @@ -34,8 +34,6 @@ type Wallet interface { Signer() Signer // IssueBaseTx creates, signs, and issues a new simple value transfer. - // Because the P-chain doesn't intend for balance transfers to occur, this - // method is expensive and abuses the creation of subnets. // // - [outputs] specifies all the recipients and amounts that should be sent // from this transaction. From 90a13f361bda434cbedcb62f515f102594cb4c79 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Wed, 6 Mar 2024 12:44:15 -0500 Subject: [PATCH 10/11] Remove put gossip (#2790) --- chains/manager.go | 30 -------- chains/subnets_test.go | 8 +-- config/config.go | 22 +----- config/config_test.go | 16 ----- config/flags.go | 6 -- config/keys.go | 6 -- message/fields.go | 13 ++-- snow/engine/common/mock_sender.go | 27 ------- snow/engine/common/no_ops_handlers.go | 22 ++---- snow/engine/common/sender.go | 11 --- snow/engine/common/test_sender.go | 35 --------- snow/engine/snowman/metrics.go | 2 - snow/engine/snowman/transitive.go | 84 +++++++--------------- snow/engine/snowman/transitive_test.go | 28 ++++---- snow/networking/router/chain_router.go | 6 +- snow/networking/sender/sender.go | 94 ------------------------- snow/networking/sender/sender_test.go | 19 ++--- snow/networking/sender/traced_sender.go | 14 ---- subnets/config.go | 11 --- utils/constants/networking.go | 18 +---- vms/platformvm/vm_test.go | 6 +- 21 files changed, 66 insertions(+), 412 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 89dc69c17c7a..7519982f18bb 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -607,16 +607,6 @@ func (m *manager) createAvalancheChain( avalancheMessageSender = sender.Trace(avalancheMessageSender, m.Tracer) } - err = m.VertexAcceptorGroup.RegisterAcceptor( - ctx.ChainID, - "gossip", - avalancheMessageSender, - false, - ) - if err != nil { // Set up the event dispatcher - return nil, fmt.Errorf("problem initializing event dispatcher: %w", err) - } - // Passes messages from the snowman engines to the network snowmanMessageSender, err := sender.New( ctx, @@ -635,16 +625,6 @@ func (m *manager) createAvalancheChain( snowmanMessageSender = sender.Trace(snowmanMessageSender, m.Tracer) } - err = m.BlockAcceptorGroup.RegisterAcceptor( - ctx.ChainID, - "gossip", - snowmanMessageSender, - false, - ) - if err != nil { // Set up the event dispatcher - return nil, fmt.Errorf("problem initializing event dispatcher: %w", err) - } - chainConfig, err := m.getChainConfig(ctx.ChainID) if err != nil { return nil, fmt.Errorf("error while fetching chain config: %w", err) @@ -1000,16 +980,6 @@ func (m *manager) createSnowmanChain( messageSender = sender.Trace(messageSender, m.Tracer) } - err = m.BlockAcceptorGroup.RegisterAcceptor( - ctx.ChainID, - "gossip", - messageSender, - false, - ) - if err != nil { // Set up the event dispatcher - return nil, fmt.Errorf("problem initializing event dispatcher: %w", err) - } - var ( bootstrapFunc func() subnetConnector = validators.UnhandledSubnetConnector diff --git a/chains/subnets_test.go b/chains/subnets_test.go index 231a8f970a15..b11ad354099e 100644 --- a/chains/subnets_test.go +++ b/chains/subnets_test.go @@ -117,16 +117,12 @@ func TestSubnetConfigs(t *testing.T) { config: map[ids.ID]subnets.Config{ constants.PrimaryNetworkID: {}, testSubnetID: { - GossipConfig: subnets.GossipConfig{ - AcceptedFrontierValidatorSize: 123456789, - }, + ValidatorOnly: true, }, }, subnetID: testSubnetID, want: subnets.Config{ - GossipConfig: subnets.GossipConfig{ - AcceptedFrontierValidatorSize: 123456789, - }, + ValidatorOnly: true, }, }, } diff --git a/config/config.go b/config/config.go index f0d8d643677b..1213115b6f16 100644 --- a/config/config.go +++ b/config/config.go @@ -53,8 +53,7 @@ const ( chainUpgradeFileName = "upgrade" subnetConfigFileExt = ".json" - keystoreDeprecationMsg = "keystore API is deprecated" - acceptedFrontierGossipDeprecationMsg = "push-based accepted frontier gossip is deprecated" + keystoreDeprecationMsg = "keystore API is deprecated" ) var ( @@ -64,13 +63,6 @@ var ( deprecatedKeys = map[string]string{ KeystoreAPIEnabledKey: keystoreDeprecationMsg, - ConsensusGossipAcceptedFrontierValidatorSizeKey: acceptedFrontierGossipDeprecationMsg, - ConsensusGossipAcceptedFrontierNonValidatorSizeKey: acceptedFrontierGossipDeprecationMsg, - ConsensusGossipAcceptedFrontierPeerSizeKey: acceptedFrontierGossipDeprecationMsg, - ConsensusGossipOnAcceptValidatorSizeKey: acceptedFrontierGossipDeprecationMsg, - ConsensusGossipOnAcceptNonValidatorSizeKey: acceptedFrontierGossipDeprecationMsg, - ConsensusGossipOnAcceptPeerSizeKey: acceptedFrontierGossipDeprecationMsg, - SnowRogueCommitThresholdKey: commitThresholdDeprecationMsg, SnowVirtuousCommitThresholdKey: commitThresholdDeprecationMsg, } @@ -257,17 +249,6 @@ func getAdaptiveTimeoutConfig(v *viper.Viper) (timer.AdaptiveTimeoutConfig, erro return config, nil } -func getGossipConfig(v *viper.Viper) subnets.GossipConfig { - return subnets.GossipConfig{ - AcceptedFrontierValidatorSize: uint(v.GetUint32(ConsensusGossipAcceptedFrontierValidatorSizeKey)), - AcceptedFrontierNonValidatorSize: uint(v.GetUint32(ConsensusGossipAcceptedFrontierNonValidatorSizeKey)), - AcceptedFrontierPeerSize: uint(v.GetUint32(ConsensusGossipAcceptedFrontierPeerSizeKey)), - OnAcceptValidatorSize: uint(v.GetUint32(ConsensusGossipOnAcceptValidatorSizeKey)), - OnAcceptNonValidatorSize: uint(v.GetUint32(ConsensusGossipOnAcceptNonValidatorSizeKey)), - OnAcceptPeerSize: uint(v.GetUint32(ConsensusGossipOnAcceptPeerSizeKey)), - } -} - func getNetworkConfig( v *viper.Viper, networkID uint32, @@ -1112,7 +1093,6 @@ func getDefaultSubnetConfig(v *viper.Viper) subnets.Config { return subnets.Config{ ConsensusParameters: getConsensusConfig(v), ValidatorOnly: false, - GossipConfig: getGossipConfig(v), ProposerMinBlockDelay: proposervm.DefaultMinBlockDelay, ProposerNumHistoricalBlocks: proposervm.DefaultNumHistoricalBlocks, } diff --git a/config/config_test.go b/config/config_test.go index 48e746647c78..820796714b72 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -20,7 +20,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowball" "github.com/ava-labs/avalanchego/subnets" - "github.com/ava-labs/avalanchego/utils/constants" ) func TestGetChainConfigsFromFiles(t *testing.T) { @@ -420,20 +419,6 @@ func TestGetSubnetConfigsFromFile(t *testing.T) { }, expectedErr: nil, }, - "gossip config": { - fileName: "2Ctt6eGAeo4MLqTmGa7AdRecuVMPGWEX9wSsCLBYrLhX4a394i.json", - givenJSON: `{"gossipOnAcceptValidatorSize": 100 }`, - testF: func(require *require.Assertions, given map[ids.ID]subnets.Config) { - id, _ := ids.FromString("2Ctt6eGAeo4MLqTmGa7AdRecuVMPGWEX9wSsCLBYrLhX4a394i") - config, ok := given[id] - require.True(ok) - require.Equal(uint(100), config.GossipConfig.OnAcceptValidatorSize) - // must still respect defaults - require.Equal(20, config.ConsensusParameters.K) - require.Equal(uint(constants.DefaultConsensusGossipOnAcceptPeerSize), config.GossipConfig.OnAcceptPeerSize) - }, - expectedErr: nil, - }, } for name, test := range tests { @@ -528,7 +513,6 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { require.Equal(20, config.ConsensusParameters.AlphaConfidence) require.Equal(30, config.ConsensusParameters.K) // must still respect defaults - require.Equal(uint(constants.DefaultConsensusGossipAcceptedFrontierPeerSize), config.GossipConfig.AcceptedFrontierPeerSize) require.Equal(256, config.ConsensusParameters.MaxOutstandingItems) }, expectedErr: nil, diff --git a/config/flags.go b/config/flags.go index 39d025dcd6bd..ac85058f6da8 100644 --- a/config/flags.go +++ b/config/flags.go @@ -183,12 +183,6 @@ func addNodeFlags(fs *pflag.FlagSet) { fs.Uint(ConsensusAppConcurrencyKey, constants.DefaultConsensusAppConcurrency, "Maximum number of goroutines to use when handling App messages on a chain") fs.Duration(ConsensusShutdownTimeoutKey, constants.DefaultConsensusShutdownTimeout, "Timeout before killing an unresponsive chain") fs.Duration(ConsensusFrontierPollFrequencyKey, constants.DefaultFrontierPollFrequency, "Frequency of polling for new consensus frontiers") - fs.Uint(ConsensusGossipAcceptedFrontierValidatorSizeKey, constants.DefaultConsensusGossipAcceptedFrontierValidatorSize, "Number of validators to gossip to when gossiping accepted frontier") - fs.Uint(ConsensusGossipAcceptedFrontierNonValidatorSizeKey, constants.DefaultConsensusGossipAcceptedFrontierNonValidatorSize, "Number of non-validators to gossip to when gossiping accepted frontier") - fs.Uint(ConsensusGossipAcceptedFrontierPeerSizeKey, constants.DefaultConsensusGossipAcceptedFrontierPeerSize, "Number of peers to gossip to when gossiping accepted frontier") - fs.Uint(ConsensusGossipOnAcceptValidatorSizeKey, constants.DefaultConsensusGossipOnAcceptValidatorSize, "Number of validators to gossip to each accepted container to") - fs.Uint(ConsensusGossipOnAcceptNonValidatorSizeKey, constants.DefaultConsensusGossipOnAcceptNonValidatorSize, "Number of non-validators to gossip to each accepted container to") - fs.Uint(ConsensusGossipOnAcceptPeerSizeKey, constants.DefaultConsensusGossipOnAcceptPeerSize, "Number of peers to gossip to each accepted container to") // Inbound Throttling fs.Uint64(InboundThrottlerAtLargeAllocSizeKey, constants.DefaultInboundThrottlerAtLargeAllocSize, "Size, in bytes, of at-large byte allocation in inbound message throttler") diff --git a/config/keys.go b/config/keys.go index 195be55434ce..28af72f59488 100644 --- a/config/keys.go +++ b/config/keys.go @@ -141,12 +141,6 @@ const ( ConsensusAppConcurrencyKey = "consensus-app-concurrency" ConsensusShutdownTimeoutKey = "consensus-shutdown-timeout" ConsensusFrontierPollFrequencyKey = "consensus-frontier-poll-frequency" - ConsensusGossipAcceptedFrontierValidatorSizeKey = "consensus-accepted-frontier-gossip-validator-size" - ConsensusGossipAcceptedFrontierNonValidatorSizeKey = "consensus-accepted-frontier-gossip-non-validator-size" - ConsensusGossipAcceptedFrontierPeerSizeKey = "consensus-accepted-frontier-gossip-peer-size" - ConsensusGossipOnAcceptValidatorSizeKey = "consensus-on-accept-gossip-validator-size" - ConsensusGossipOnAcceptNonValidatorSizeKey = "consensus-on-accept-gossip-non-validator-size" - ConsensusGossipOnAcceptPeerSizeKey = "consensus-on-accept-gossip-peer-size" ProposerVMUseCurrentHeightKey = "proposervm-use-current-height" FdLimitKey = "fd-limit" IndexEnabledKey = "index-enabled" diff --git a/message/fields.go b/message/fields.go index 08e744fab911..94dcf2a23061 100644 --- a/message/fields.go +++ b/message/fields.go @@ -9,7 +9,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/proto/pb/p2p" - "github.com/ava-labs/avalanchego/utils/constants" ) var ( @@ -106,14 +105,10 @@ func GetRequestID(m any) (uint32, bool) { return requestID, true } - // AppGossip is the only message currently not containing a requestID - // Here we assign the requestID already in use for gossiped containers - // to allow a uniform handling of all messages - if _, ok := m.(*p2p.AppGossip); ok { - return constants.GossipMsgRequestID, true - } - - return 0, false + // AppGossip is the only inbound message not containing a requestID. For + // ease of handling, imagine that it does have a requestID. + _, ok := m.(*p2p.AppGossip) + return 0, ok } type engineTypeGetter interface { diff --git a/snow/engine/common/mock_sender.go b/snow/engine/common/mock_sender.go index 8694cb4b3191..ecc4a4f851b3 100644 --- a/snow/engine/common/mock_sender.go +++ b/snow/engine/common/mock_sender.go @@ -14,7 +14,6 @@ import ( reflect "reflect" ids "github.com/ava-labs/avalanchego/ids" - snow "github.com/ava-labs/avalanchego/snow" set "github.com/ava-labs/avalanchego/utils/set" gomock "go.uber.org/mock/gomock" ) @@ -42,20 +41,6 @@ func (m *MockSender) EXPECT() *MockSenderMockRecorder { return m.recorder } -// Accept mocks base method. -func (m *MockSender) Accept(ctx *snow.ConsensusContext, containerID ids.ID, container []byte) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Accept", ctx, containerID, container) - ret0, _ := ret[0].(error) - return ret0 -} - -// Accept indicates an expected call of Accept. -func (mr *MockSenderMockRecorder) Accept(ctx, containerID, container any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Accept", reflect.TypeOf((*MockSender)(nil).Accept), ctx, containerID, container) -} - // SendAccepted mocks base method. func (m *MockSender) SendAccepted(ctx context.Context, nodeID ids.NodeID, requestID uint32, containerIDs []ids.ID) { m.ctrl.T.Helper() @@ -300,18 +285,6 @@ func (mr *MockSenderMockRecorder) SendGetStateSummaryFrontier(ctx, nodeIDs, requ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendGetStateSummaryFrontier", reflect.TypeOf((*MockSender)(nil).SendGetStateSummaryFrontier), ctx, nodeIDs, requestID) } -// SendGossip mocks base method. -func (m *MockSender) SendGossip(ctx context.Context, container []byte) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SendGossip", ctx, container) -} - -// SendGossip indicates an expected call of SendGossip. -func (mr *MockSenderMockRecorder) SendGossip(ctx, container any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendGossip", reflect.TypeOf((*MockSender)(nil).SendGossip), ctx, container) -} - // SendPullQuery mocks base method. func (m *MockSender) SendPullQuery(ctx context.Context, nodeIDs set.Set[ids.NodeID], requestID uint32, containerID ids.ID, requestedHeight uint64) { m.ctrl.T.Helper() diff --git a/snow/engine/common/no_ops_handlers.go b/snow/engine/common/no_ops_handlers.go index 870c6694a7a7..4458070e7696 100644 --- a/snow/engine/common/no_ops_handlers.go +++ b/snow/engine/common/no_ops_handlers.go @@ -11,7 +11,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/message" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/version" @@ -180,21 +179,12 @@ func NewNoOpPutHandler(log logging.Logger) PutHandler { } func (nop *noOpPutHandler) Put(_ context.Context, nodeID ids.NodeID, requestID uint32, _ []byte) error { - if requestID == constants.GossipMsgRequestID { - nop.log.Verbo("dropping request", - zap.String("reason", "unhandled by this gear"), - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - ) - } else { - nop.log.Debug("dropping request", - zap.String("reason", "unhandled by this gear"), - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - ) - } + nop.log.Debug("dropping request", + zap.String("reason", "unhandled by this gear"), + zap.Stringer("messageOp", message.PutOp), + zap.Stringer("nodeID", nodeID), + zap.Uint32("requestID", requestID), + ) return nil } diff --git a/snow/engine/common/sender.go b/snow/engine/common/sender.go index 8b9a8394d0a8..adb4be513c6f 100644 --- a/snow/engine/common/sender.go +++ b/snow/engine/common/sender.go @@ -7,7 +7,6 @@ import ( "context" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/set" ) @@ -35,15 +34,12 @@ import ( // time the requestID space has been exhausted, the beginning of the requestID // space is free of conflicts. type Sender interface { - snow.Acceptor - StateSummarySender AcceptedStateSummarySender FrontierSender AcceptedSender FetchSender QuerySender - Gossiper AppSender } @@ -160,13 +156,6 @@ type QuerySender interface { ) } -// Gossiper defines how a consensus engine gossips a container on the accepted -// frontier to other nodes -type Gossiper interface { - // Gossip the provided container throughout the network - SendGossip(ctx context.Context, container []byte) -} - // NetworkAppSender sends VM-level messages to nodes in the network. type NetworkAppSender interface { // Send an application-level request. diff --git a/snow/engine/common/test_sender.go b/snow/engine/common/test_sender.go index f6b69f70c03d..cbec25f33f67 100644 --- a/snow/engine/common/test_sender.go +++ b/snow/engine/common/test_sender.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/set" ) @@ -18,7 +17,6 @@ var ( _ Sender = (*SenderTest)(nil) _ AppSender = (*FakeSender)(nil) - errAccept = errors.New("unexpectedly called Accept") errSendAppRequest = errors.New("unexpectedly called SendAppRequest") errSendAppResponse = errors.New("unexpectedly called SendAppResponse") errSendAppError = errors.New("unexpectedly called SendAppError") @@ -30,19 +28,16 @@ var ( type SenderTest struct { T require.TestingT - CantAccept, CantSendGetStateSummaryFrontier, CantSendStateSummaryFrontier, CantSendGetAcceptedStateSummary, CantSendAcceptedStateSummary, CantSendGetAcceptedFrontier, CantSendAcceptedFrontier, CantSendGetAccepted, CantSendAccepted, CantSendGet, CantSendGetAncestors, CantSendPut, CantSendAncestors, CantSendPullQuery, CantSendPushQuery, CantSendChits, - CantSendGossip, CantSendAppRequest, CantSendAppResponse, CantSendAppError, CantSendAppGossip, CantSendAppGossipSpecific, CantSendCrossChainAppRequest, CantSendCrossChainAppResponse, CantSendCrossChainAppError bool - AcceptF func(*snow.ConsensusContext, ids.ID, []byte) error SendGetStateSummaryFrontierF func(context.Context, set.Set[ids.NodeID], uint32) SendStateSummaryFrontierF func(context.Context, ids.NodeID, uint32, []byte) SendGetAcceptedStateSummaryF func(context.Context, set.Set[ids.NodeID], uint32, []uint64) @@ -58,7 +53,6 @@ type SenderTest struct { SendPushQueryF func(context.Context, set.Set[ids.NodeID], uint32, []byte, uint64) SendPullQueryF func(context.Context, set.Set[ids.NodeID], uint32, ids.ID, uint64) SendChitsF func(context.Context, ids.NodeID, uint32, ids.ID, ids.ID, ids.ID) - SendGossipF func(context.Context, []byte) SendAppRequestF func(context.Context, set.Set[ids.NodeID], uint32, []byte) error SendAppResponseF func(context.Context, ids.NodeID, uint32, []byte) error SendAppErrorF func(context.Context, ids.NodeID, uint32, int32, string) error @@ -71,7 +65,6 @@ type SenderTest struct { // Default set the default callable value to [cant] func (s *SenderTest) Default(cant bool) { - s.CantAccept = cant s.CantSendGetStateSummaryFrontier = cant s.CantSendStateSummaryFrontier = cant s.CantSendGetAcceptedStateSummary = cant @@ -87,7 +80,6 @@ func (s *SenderTest) Default(cant bool) { s.CantSendPullQuery = cant s.CantSendPushQuery = cant s.CantSendChits = cant - s.CantSendGossip = cant s.CantSendAppRequest = cant s.CantSendAppResponse = cant s.CantSendAppGossip = cant @@ -96,22 +88,6 @@ func (s *SenderTest) Default(cant bool) { s.CantSendCrossChainAppResponse = cant } -// Accept calls AcceptF if it was initialized. If it wasn't initialized and this -// function shouldn't be called and testing was initialized, then testing will -// fail. -func (s *SenderTest) Accept(ctx *snow.ConsensusContext, containerID ids.ID, container []byte) error { - if s.AcceptF != nil { - return s.AcceptF(ctx, containerID, container) - } - if !s.CantAccept { - return nil - } - if s.T != nil { - require.FailNow(s.T, errAccept.Error()) - } - return errAccept -} - // SendGetStateSummaryFrontier calls SendGetStateSummaryFrontierF if it was // initialized. If it wasn't initialized and this function shouldn't be called // and testing was initialized, then testing will fail. @@ -277,17 +253,6 @@ func (s *SenderTest) SendChits(ctx context.Context, vdr ids.NodeID, requestID ui } } -// SendGossip calls SendGossipF if it was initialized. If it wasn't initialized -// and this function shouldn't be called and testing was initialized, then -// testing will fail. -func (s *SenderTest) SendGossip(ctx context.Context, container []byte) { - if s.SendGossipF != nil { - s.SendGossipF(ctx, container) - } else if s.CantSendGossip && s.T != nil { - require.FailNow(s.T, "Unexpectedly called SendGossip") - } -} - // SendCrossChainAppRequest calls SendCrossChainAppRequestF if it was // initialized. If it wasn't initialized and this function shouldn't be called // and testing was initialized, then testing will fail. diff --git a/snow/engine/snowman/metrics.go b/snow/engine/snowman/metrics.go index 5dd65d8afa14..1ee31cdf1dc9 100644 --- a/snow/engine/snowman/metrics.go +++ b/snow/engine/snowman/metrics.go @@ -13,7 +13,6 @@ import ( const ( pullGossipSource = "pull_gossip" pushGossipSource = "push_gossip" - putGossipSource = "put_gossip" builtSource = "built" unknownSource = "unknown" ) @@ -141,7 +140,6 @@ func (m *metrics) Initialize(namespace string, reg prometheus.Registerer) error // Register the labels m.issued.WithLabelValues(pullGossipSource) m.issued.WithLabelValues(pushGossipSource) - m.issued.WithLabelValues(putGossipSource) m.issued.WithLabelValues(builtSource) m.issued.WithLabelValues(unknownSource) diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index a1335a1ad7fd..a28bdcd35fcd 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -33,14 +33,7 @@ import ( "github.com/ava-labs/avalanchego/utils/wrappers" ) -const ( - nonVerifiedCacheSize = 64 * units.MiB - - // putGossipPeriod specifies the number of times Gossip will be called per - // Put gossip. This is done to avoid splitting Gossip into multiple - // functions and to allow more frequent pull gossip than push gossip. - putGossipPeriod = 10 -) +const nonVerifiedCacheSize = 64 * units.MiB var _ common.Engine = (*Transitive)(nil) @@ -65,8 +58,6 @@ type Transitive struct { requestID uint32 - gossipCounter int - // track outstanding preference requests polls poll.Set @@ -157,41 +148,7 @@ func New(config Config) (*Transitive, error) { func (t *Transitive) Gossip(ctx context.Context) error { lastAcceptedID, lastAcceptedHeight := t.Consensus.LastAccepted() - if numProcessing := t.Consensus.NumProcessing(); numProcessing == 0 { - t.Ctx.Log.Verbo("sampling from validators", - zap.Stringer("validators", t.Validators), - ) - - // Uniform sampling is used here to reduce bandwidth requirements of - // nodes with a large amount of stake weight. - vdrID, ok := t.ConnectedValidators.SampleValidator() - if !ok { - t.Ctx.Log.Warn("skipping block gossip", - zap.String("reason", "no connected validators"), - ) - return nil - } - - nextHeightToAccept, err := math.Add64(lastAcceptedHeight, 1) - if err != nil { - t.Ctx.Log.Error("skipping block gossip", - zap.String("reason", "block height overflow"), - zap.Stringer("blkID", lastAcceptedID), - zap.Uint64("lastAcceptedHeight", lastAcceptedHeight), - zap.Error(err), - ) - return nil - } - - t.requestID++ - t.Sender.SendPullQuery( - ctx, - set.Of(vdrID), - t.requestID, - t.Consensus.Preference(), - nextHeightToAccept, - ) - } else { + if numProcessing := t.Consensus.NumProcessing(); numProcessing != 0 { t.Ctx.Log.Debug("skipping block gossip", zap.String("reason", "blocks currently processing"), zap.Int("numProcessing", numProcessing), @@ -201,28 +158,42 @@ func (t *Transitive) Gossip(ctx context.Context) error { // when attempting to issue a query. This can happen if a subnet was // temporarily misconfigured and there were no validators. t.repoll(ctx) + return nil } - // TODO: Remove periodic push gossip after v1.11.x is activated - t.gossipCounter++ - t.gossipCounter %= putGossipPeriod - if t.gossipCounter > 0 { + t.Ctx.Log.Verbo("sampling from validators", + zap.Stringer("validators", t.Validators), + ) + + // Uniform sampling is used here to reduce bandwidth requirements of + // nodes with a large amount of stake weight. + vdrID, ok := t.ConnectedValidators.SampleValidator() + if !ok { + t.Ctx.Log.Warn("skipping block gossip", + zap.String("reason", "no connected validators"), + ) return nil } - lastAccepted, err := t.getBlock(ctx, lastAcceptedID) + nextHeightToAccept, err := math.Add64(lastAcceptedHeight, 1) if err != nil { - t.Ctx.Log.Warn("dropping gossip request", - zap.String("reason", "block couldn't be loaded"), + t.Ctx.Log.Error("skipping block gossip", + zap.String("reason", "block height overflow"), zap.Stringer("blkID", lastAcceptedID), + zap.Uint64("lastAcceptedHeight", lastAcceptedHeight), zap.Error(err), ) return nil } - t.Ctx.Log.Verbo("gossiping accepted block to the network", - zap.Stringer("blkID", lastAcceptedID), + + t.requestID++ + t.Sender.SendPullQuery( + ctx, + set.Of(vdrID), + t.requestID, + t.Consensus.Preference(), + nextHeightToAccept, ) - t.Sender.SendGossip(ctx, lastAccepted.Bytes()) return nil } @@ -272,8 +243,6 @@ func (t *Transitive) Put(ctx context.Context, nodeID ids.NodeID, requestID uint3 } issuedMetric = t.blkReqSourceMetric[req] - case requestID == constants.GossipMsgRequestID: - issuedMetric = t.metrics.issued.WithLabelValues(putGossipSource) default: // This can happen if this block was provided to this engine while a Get // request was outstanding. For example, the block may have been locally @@ -554,7 +523,6 @@ func (t *Transitive) HealthCheck(ctx context.Context) (interface{}, error) { t.Ctx.Log.Verbo("running health check", zap.Uint32("requestID", t.requestID), - zap.Int("gossipCounter", t.gossipCounter), zap.Stringer("polls", t.polls), zap.Reflect("outstandingBlockRequests", t.blkReqs), zap.Stringer("blockedJobs", &t.blocked), diff --git a/snow/engine/snowman/transitive_test.go b/snow/engine/snowman/transitive_test.go index 944e0f2a24e7..bd5966f797b9 100644 --- a/snow/engine/snowman/transitive_test.go +++ b/snow/engine/snowman/transitive_test.go @@ -20,7 +20,6 @@ import ( "github.com/ava-labs/avalanchego/snow/engine/snowman/block" "github.com/ava-labs/avalanchego/snow/engine/snowman/getter" "github.com/ava-labs/avalanchego/snow/validators" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/version" ) @@ -2426,13 +2425,15 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { require.Equal(vdr, inVdr) *asked = true } + sender.CantSendChits = false + // This engine receives a Gossip message for [blk2] which was "unknown" in this engine. // The engine thus learns about its ancestor [blk1] and should send a Get request for it. // (see above for expected "Get" request) - require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes())) + require.NoError(te.PushQuery(context.Background(), vdr, 0, blk2.Bytes(), 0)) require.True(*asked) - // Prepare to PushQuery [blk1] after our Get request is fulfilled. We should not PushQuery + // Prepare to PullQuery [blk1] after our Get request is fulfilled. We should not PullQuery // [blk2] since it currently fails verification. queried := new(bool) queryRequestID := new(uint32) @@ -2448,8 +2449,8 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { } // This engine now handles the response to the "Get" request. This should cause [blk1] to be issued // which will result in attempting to issue [blk2]. However, [blk2] should fail verification and be dropped. - // By issuing [blk1], this node should fire a "PushQuery" request for [blk1]. - // (see above for expected "PushQuery" request) + // By issuing [blk1], this node should fire a "PullQuery" request for [blk1]. + // (see above for expected "PullQuery" request) require.NoError(te.Put(context.Background(), vdr, *reqID, blk1.Bytes())) require.True(*asked) require.True(*queried, "Didn't query the newly issued blk1") @@ -2503,8 +2504,9 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { return nil, errUnknownBlock } } + *queried = false - // Prepare to PushQuery [blk2] after receiving a Gossip message with [blk2]. + // Prepare to PullQuery [blk2] after receiving a Gossip message with [blk2]. sender.SendPullQueryF = func(_ context.Context, inVdrs set.Set[ids.NodeID], requestID uint32, blkID ids.ID, requestedHeight uint64) { require.False(*queried) *queried = true @@ -2513,8 +2515,8 @@ func TestEngineBubbleVotesThroughInvalidBlock(t *testing.T) { require.Equal(blk2.ID(), blkID) require.Equal(uint64(2), requestedHeight) } - // Expect that the Engine will send a PushQuery after receiving this Gossip message for [blk2]. - require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk2.Bytes())) + // Expect that the Engine will send a PullQuery after receiving this Gossip message for [blk2]. + require.NoError(te.PushQuery(context.Background(), vdr, 0, blk2.Bytes(), 0)) require.True(*queried) // After a single vote for [blk2], it should be marked as accepted. @@ -2611,14 +2613,16 @@ func TestEngineBubbleVotesThroughInvalidChain(t *testing.T) { require.Equal(vdr, inVdr) *asked = true } + sender.CantSendChits = false + // Receive Gossip message for [blk3] first and expect the sender to issue a // Get request for its ancestor: [blk2]. - require.NoError(te.Put(context.Background(), vdr, constants.GossipMsgRequestID, blk3.Bytes())) + require.NoError(te.PushQuery(context.Background(), vdr, 0, blk3.Bytes(), 0)) require.True(*asked) - // Prepare to PushQuery [blk1] after our request for [blk2] is fulfilled. - // We should not PushQuery [blk2] since it currently fails verification. - // We should not PushQuery [blk3] because [blk2] wasn't issued. + // Prepare to PullQuery [blk1] after our request for [blk2] is fulfilled. + // We should not PullQuery [blk2] since it currently fails verification. + // We should not PullQuery [blk3] because [blk2] wasn't issued. queried := new(bool) queryRequestID := new(uint32) sender.SendPullQueryF = func(_ context.Context, inVdrs set.Set[ids.NodeID], requestID uint32, blkID ids.ID, requestedHeight uint64) { diff --git a/snow/networking/router/chain_router.go b/snow/networking/router/chain_router.go index 9c2425883b14..2553bef7d1f7 100644 --- a/snow/networking/router/chain_router.go +++ b/snow/networking/router/chain_router.go @@ -270,11 +270,7 @@ func (cr *ChainRouter) HandleInbound(ctx context.Context, msg message.InboundMes } chainCtx := chain.Context() - - // TODO: [requestID] can overflow, which means a timeout on the request - // before the overflow may not be handled properly. - if notRequested := message.UnrequestedOps.Contains(op); notRequested || - (op == message.PutOp && requestID == constants.GossipMsgRequestID) { + if message.UnrequestedOps.Contains(op) { if chainCtx.Executing.Get() { cr.log.Debug("dropping message and skipping queue", zap.String("reason", "the chain is currently executing"), diff --git a/snow/networking/sender/sender.go b/snow/networking/sender/sender.go index 4bc6325a539d..eeef6f722480 100644 --- a/snow/networking/sender/sender.go +++ b/snow/networking/sender/sender.go @@ -18,7 +18,6 @@ import ( "github.com/ava-labs/avalanchego/snow/networking/router" "github.com/ava-labs/avalanchego/snow/networking/timeout" "github.com/ava-labs/avalanchego/subnets" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" ) @@ -1610,96 +1609,3 @@ func (s *sender) SendAppGossip( } return nil } - -func (s *sender) SendGossip(_ context.Context, container []byte) { - // Create the outbound message. - outMsg, err := s.msgCreator.Put( - s.ctx.ChainID, - constants.GossipMsgRequestID, - container, - s.engineType, - ) - if err != nil { - s.ctx.Log.Error("failed to build message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - zap.Binary("container", container), - zap.Error(err), - ) - return - } - - gossipConfig := s.subnet.Config().GossipConfig - sentTo := s.sender.Gossip( - outMsg, - s.ctx.SubnetID, - int(gossipConfig.AcceptedFrontierValidatorSize), - int(gossipConfig.AcceptedFrontierNonValidatorSize), - int(gossipConfig.AcceptedFrontierPeerSize), - s.subnet, - ) - if sentTo.Len() == 0 { - if s.ctx.Log.Enabled(logging.Verbo) { - s.ctx.Log.Verbo("failed to send message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - zap.Binary("container", container), - ) - } else { - s.ctx.Log.Debug("failed to send message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - ) - } - } -} - -// Accept is called after every consensus decision -func (s *sender) Accept(ctx *snow.ConsensusContext, _ ids.ID, container []byte) error { - if ctx.State.Get().State != snow.NormalOp { - // don't gossip during bootstrapping - return nil - } - - // Create the outbound message. - outMsg, err := s.msgCreator.Put( - s.ctx.ChainID, - constants.GossipMsgRequestID, - container, - s.engineType, - ) - if err != nil { - s.ctx.Log.Error("failed to build message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - zap.Binary("container", container), - zap.Error(err), - ) - return nil - } - - gossipConfig := s.subnet.Config().GossipConfig - sentTo := s.sender.Gossip( - outMsg, - s.ctx.SubnetID, - int(gossipConfig.OnAcceptValidatorSize), - int(gossipConfig.OnAcceptNonValidatorSize), - int(gossipConfig.OnAcceptPeerSize), - s.subnet, - ) - if sentTo.Len() == 0 { - if s.ctx.Log.Enabled(logging.Verbo) { - s.ctx.Log.Verbo("failed to send message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - zap.Binary("container", container), - ) - } else { - s.ctx.Log.Debug("failed to send message", - zap.Stringer("messageOp", message.PutOp), - zap.Stringer("chainID", s.ctx.ChainID), - ) - } - } - return nil -} diff --git a/snow/networking/sender/sender_test.go b/snow/networking/sender/sender_test.go index 3fd9598f70c1..3f369c26ef4e 100644 --- a/snow/networking/sender/sender_test.go +++ b/snow/networking/sender/sender_test.go @@ -40,13 +40,6 @@ import ( const testThreadPoolSize = 2 -var defaultSubnetConfig = subnets.Config{ - GossipConfig: subnets.GossipConfig{ - AcceptedFrontierPeerSize: 2, - OnAcceptPeerSize: 2, - }, -} - func TestTimeout(t *testing.T) { require := require.New(t) @@ -106,7 +99,7 @@ func TestTimeout(t *testing.T) { &chainRouter, tm, p2p.EngineType_ENGINE_TYPE_SNOWMAN, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) @@ -372,7 +365,7 @@ func TestReliableMessages(t *testing.T) { &chainRouter, tm, p2p.EngineType_ENGINE_TYPE_SNOWMAN, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) @@ -518,7 +511,7 @@ func TestReliableMessagesToMyself(t *testing.T) { &chainRouter, tm, p2p.EngineType_ENGINE_TYPE_SNOWMAN, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) @@ -820,7 +813,7 @@ func TestSender_Bootstrap_Requests(t *testing.T) { router, timeoutManager, engineType, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) @@ -1032,7 +1025,7 @@ func TestSender_Bootstrap_Responses(t *testing.T) { router, timeoutManager, engineType, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) @@ -1192,7 +1185,7 @@ func TestSender_Single_Request(t *testing.T) { router, timeoutManager, engineType, - subnets.New(ctx.NodeID, defaultSubnetConfig), + subnets.New(ctx.NodeID, subnets.Config{}), ) require.NoError(err) diff --git a/snow/networking/sender/traced_sender.go b/snow/networking/sender/traced_sender.go index 096bc15d0595..5157b59c8b32 100644 --- a/snow/networking/sender/traced_sender.go +++ b/snow/networking/sender/traced_sender.go @@ -9,7 +9,6 @@ import ( "go.opentelemetry.io/otel/attribute" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/trace" "github.com/ava-labs/avalanchego/utils/set" @@ -291,16 +290,3 @@ func (s *tracedSender) SendAppGossip( numPeers, ) } - -func (s *tracedSender) SendGossip(ctx context.Context, container []byte) { - _, span := s.tracer.Start(ctx, "tracedSender.SendGossip", oteltrace.WithAttributes( - attribute.Int("containerLen", len(container)), - )) - defer span.End() - - s.sender.SendGossip(ctx, container) -} - -func (s *tracedSender) Accept(ctx *snow.ConsensusContext, containerID ids.ID, container []byte) error { - return s.sender.Accept(ctx, containerID, container) -} diff --git a/subnets/config.go b/subnets/config.go index 2fa86093fa2a..f5bd223b0705 100644 --- a/subnets/config.go +++ b/subnets/config.go @@ -15,18 +15,7 @@ import ( var errAllowedNodesWhenNotValidatorOnly = errors.New("allowedNodes can only be set when ValidatorOnly is true") -type GossipConfig struct { - AcceptedFrontierValidatorSize uint `json:"gossipAcceptedFrontierValidatorSize" yaml:"gossipAcceptedFrontierValidatorSize"` - AcceptedFrontierNonValidatorSize uint `json:"gossipAcceptedFrontierNonValidatorSize" yaml:"gossipAcceptedFrontierNonValidatorSize"` - AcceptedFrontierPeerSize uint `json:"gossipAcceptedFrontierPeerSize" yaml:"gossipAcceptedFrontierPeerSize"` - OnAcceptValidatorSize uint `json:"gossipOnAcceptValidatorSize" yaml:"gossipOnAcceptValidatorSize"` - OnAcceptNonValidatorSize uint `json:"gossipOnAcceptNonValidatorSize" yaml:"gossipOnAcceptNonValidatorSize"` - OnAcceptPeerSize uint `json:"gossipOnAcceptPeerSize" yaml:"gossipOnAcceptPeerSize"` -} - type Config struct { - GossipConfig - // ValidatorOnly indicates that this Subnet's Chains are available to only subnet validators. // No chain related messages will go out to non-validators. // Validators will drop messages received from non-validators. diff --git a/utils/constants/networking.go b/utils/constants/networking.go index 3b8622e5d836..6982671a2c15 100644 --- a/utils/constants/networking.go +++ b/utils/constants/networking.go @@ -4,19 +4,13 @@ package constants import ( - "math" "time" "github.com/ava-labs/avalanchego/utils/compression" "github.com/ava-labs/avalanchego/utils/units" ) -// Const variables to be exported const ( - // Request ID used when sending a Put message to gossip an accepted container - // (ie not sent in response to a Get) - GossipMsgRequestID uint32 = math.MaxUint32 - // The network must be "tcp", "tcp4", "tcp6", "unix" or "unixpacket". NetworkType = "tcp" @@ -73,15 +67,9 @@ const ( DefaultBenchlistMinFailingDuration = 2*time.Minute + 30*time.Second // Router - DefaultConsensusAppConcurrency = 2 - DefaultConsensusShutdownTimeout = time.Minute - DefaultFrontierPollFrequency = 100 * time.Millisecond - DefaultConsensusGossipAcceptedFrontierValidatorSize = 0 - DefaultConsensusGossipAcceptedFrontierNonValidatorSize = 0 - DefaultConsensusGossipAcceptedFrontierPeerSize = 1 - DefaultConsensusGossipOnAcceptValidatorSize = 0 - DefaultConsensusGossipOnAcceptNonValidatorSize = 0 - DefaultConsensusGossipOnAcceptPeerSize = 10 + DefaultConsensusAppConcurrency = 2 + DefaultConsensusShutdownTimeout = time.Minute + DefaultFrontierPollFrequency = 100 * time.Millisecond // Inbound Throttling DefaultInboundThrottlerAtLargeAllocSize = 6 * units.MiB diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 81329eca3c06..23c1c45ba7f4 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -1375,10 +1375,6 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { externalSender.Default(true) // Passes messages from the consensus engine to the network - gossipConfig := subnets.GossipConfig{ - AcceptedFrontierPeerSize: 1, - OnAcceptPeerSize: 1, - } sender, err := sender.New( consensusCtx, mc, @@ -1386,7 +1382,7 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { chainRouter, timeoutManager, p2p.EngineType_ENGINE_TYPE_SNOWMAN, - subnets.New(consensusCtx.NodeID, subnets.Config{GossipConfig: gossipConfig}), + subnets.New(consensusCtx.NodeID, subnets.Config{}), ) require.NoError(err) From 639b9ca37034762143c712f4f11f48ad788a5a6f Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Wed, 6 Mar 2024 13:08:15 -0500 Subject: [PATCH 11/11] [vms/platformvm] Remove `GetPendingValidators` API (#2817) --- vms/platformvm/client.go | 19 +----- vms/platformvm/service.go | 122 -------------------------------------- 2 files changed, 1 insertion(+), 140 deletions(-) diff --git a/vms/platformvm/client.go b/vms/platformvm/client.go index a2720e417b80..34c5eecbe85a 100644 --- a/vms/platformvm/client.go +++ b/vms/platformvm/client.go @@ -68,8 +68,6 @@ type Client interface { GetStakingAssetID(ctx context.Context, subnetID ids.ID, options ...rpc.Option) (ids.ID, error) // GetCurrentValidators returns the list of current validators for subnet with ID [subnetID] GetCurrentValidators(ctx context.Context, subnetID ids.ID, nodeIDs []ids.NodeID, options ...rpc.Option) ([]ClientPermissionlessValidator, error) - // GetPendingValidators returns the list of pending validators for subnet with ID [subnetID] - GetPendingValidators(ctx context.Context, subnetID ids.ID, nodeIDs []ids.NodeID, options ...rpc.Option) ([]interface{}, []interface{}, error) // GetCurrentSupply returns an upper bound on the supply of AVAX in the system along with the P-chain height GetCurrentSupply(ctx context.Context, subnetID ids.ID, options ...rpc.Option) (uint64, uint64, error) // SampleValidators returns the nodeIDs of a sample of [sampleSize] validators from the current validator set for subnet with ID [subnetID] @@ -103,8 +101,7 @@ type Client interface { // GetStake returns the amount of nAVAX that [addrs] have cumulatively // staked on the Primary Network. // - // Deprecated: Stake should be calculated using GetTx, GetCurrentValidators, - // and GetPendingValidators. + // Deprecated: Stake should be calculated using GetTx and GetCurrentValidators. GetStake( ctx context.Context, addrs []ids.ShortID, @@ -325,20 +322,6 @@ func (c *client) GetCurrentValidators( return getClientPermissionlessValidators(res.Validators) } -func (c *client) GetPendingValidators( - ctx context.Context, - subnetID ids.ID, - nodeIDs []ids.NodeID, - options ...rpc.Option, -) ([]interface{}, []interface{}, error) { - res := &GetPendingValidatorsReply{} - err := c.requester.SendRequest(ctx, "platform.getPendingValidators", &GetPendingValidatorsArgs{ - SubnetID: subnetID, - NodeIDs: nodeIDs, - }, res, options...) - return res.Validators, res.Delegators, err -} - func (c *client) GetCurrentSupply(ctx context.Context, subnetID ids.ID, options ...rpc.Option) (uint64, uint64, error) { res := &GetCurrentSupplyReply{} err := c.requester.SendRequest(ctx, "platform.getCurrentSupply", &GetCurrentSupplyArgs{ diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 5addf50a3879..706fe8daced1 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -932,128 +932,6 @@ func (s *Service) GetCurrentValidators(_ *http.Request, args *GetCurrentValidato return nil } -// GetPendingValidatorsArgs are the arguments for calling GetPendingValidators -type GetPendingValidatorsArgs struct { - // Subnet we're getting the pending validators of - // If omitted, defaults to primary network - SubnetID ids.ID `json:"subnetID"` - // NodeIDs of validators to request. If [NodeIDs] - // is empty, it fetches all pending validators. If - // some requested nodeIDs are not pending validators, - // they are omitted from the response. - NodeIDs []ids.NodeID `json:"nodeIDs"` -} - -// GetPendingValidatorsReply are the results from calling GetPendingValidators. -type GetPendingValidatorsReply struct { - Validators []interface{} `json:"validators"` - Delegators []interface{} `json:"delegators"` -} - -// GetPendingValidators returns the lists of pending validators and delegators. -func (s *Service) GetPendingValidators(_ *http.Request, args *GetPendingValidatorsArgs, reply *GetPendingValidatorsReply) error { - s.vm.ctx.Log.Debug("API called", - zap.String("service", "platform"), - zap.String("method", "getPendingValidators"), - ) - - reply.Validators = []interface{}{} - reply.Delegators = []interface{}{} - - // Create set of nodeIDs - nodeIDs := set.Of(args.NodeIDs...) - - s.vm.ctx.Lock.Lock() - defer s.vm.ctx.Lock.Unlock() - - numNodeIDs := nodeIDs.Len() - targetStakers := make([]*state.Staker, 0, numNodeIDs) - if numNodeIDs == 0 { // Include all nodes - pendingStakerIterator, err := s.vm.state.GetPendingStakerIterator() - if err != nil { - return err - } - for pendingStakerIterator.Next() { // Iterates in order of increasing stop time - staker := pendingStakerIterator.Value() - if args.SubnetID != staker.SubnetID { - continue - } - targetStakers = append(targetStakers, staker) - } - pendingStakerIterator.Release() - } else { - for nodeID := range nodeIDs { - staker, err := s.vm.state.GetPendingValidator(args.SubnetID, nodeID) - switch err { - case nil: - case database.ErrNotFound: - // nothing to do, continue - continue - default: - return err - } - targetStakers = append(targetStakers, staker) - - delegatorsIt, err := s.vm.state.GetPendingDelegatorIterator(args.SubnetID, nodeID) - if err != nil { - return err - } - for delegatorsIt.Next() { - staker := delegatorsIt.Value() - targetStakers = append(targetStakers, staker) - } - delegatorsIt.Release() - } - } - - for _, pendingStaker := range targetStakers { - nodeID := pendingStaker.NodeID - weight := avajson.Uint64(pendingStaker.Weight) - apiStaker := platformapi.Staker{ - TxID: pendingStaker.TxID, - NodeID: nodeID, - StartTime: avajson.Uint64(pendingStaker.StartTime.Unix()), - EndTime: avajson.Uint64(pendingStaker.EndTime.Unix()), - Weight: weight, - StakeAmount: &weight, - } - - switch pendingStaker.Priority { - case txs.PrimaryNetworkValidatorPendingPriority, txs.SubnetPermissionlessValidatorPendingPriority: - attr, err := s.loadStakerTxAttributes(pendingStaker.TxID) - if err != nil { - return err - } - - shares := attr.shares - delegationFee := avajson.Float32(100 * float32(shares) / float32(reward.PercentDenominator)) - - connected := s.vm.uptimeManager.IsConnected(nodeID, args.SubnetID) - vdr := platformapi.PermissionlessValidator{ - Staker: apiStaker, - DelegationFee: delegationFee, - Connected: connected, - Signer: attr.proofOfPossession, - } - reply.Validators = append(reply.Validators, vdr) - - case txs.PrimaryNetworkDelegatorApricotPendingPriority, txs.PrimaryNetworkDelegatorBanffPendingPriority, txs.SubnetPermissionlessDelegatorPendingPriority: - reply.Delegators = append(reply.Delegators, apiStaker) - - case txs.SubnetPermissionedValidatorPendingPriority: - connected := s.vm.uptimeManager.IsConnected(nodeID, args.SubnetID) - reply.Validators = append(reply.Validators, platformapi.PermissionedValidator{ - Staker: apiStaker, - Connected: connected, - }) - - default: - return fmt.Errorf("unexpected staker priority %d", pendingStaker.Priority) - } - } - return nil -} - // GetCurrentSupplyArgs are the arguments for calling GetCurrentSupply type GetCurrentSupplyArgs struct { SubnetID ids.ID `json:"subnetID"`