Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup ids.NodeID usage #2280

Merged
merged 6 commits into from
Nov 9, 2023
Merged

Cleanup ids.NodeID usage #2280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f27c18c
improved usage of NodeIDs methods
abi87 Nov 9, 2023
6f35ae0
nit
abi87 Nov 9, 2023
fc57aef
introduced BuildTestNodeID
abi87 Nov 9, 2023
911ae39
spread usage of BuildTestNodeID
abi87 Nov 9, 2023
346088f
leftovers
abi87 Nov 9, 2023
6e6a5fe
nits
abi87 Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ids/test_generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,12 @@ func GenerateTestShortID() ShortID {
func GenerateTestNodeID() NodeID {
return NodeID(GenerateTestShortID())
}

// BuildTestNodeID is an utility to build NodeID from bytes in UTs
// It must not be used in production code. In production code we should
// use ToNodeID, which performs proper length checking.
func BuildTestNodeID(src []byte) NodeID {
res := NodeID{}
copy(res[:], src)
return res
}
16 changes: 8 additions & 8 deletions network/peer/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,24 @@ func TestSet(t *testing.T) {
set := NewSet()

peer1 := &peer{
id: ids.NodeID{0x01},
id: ids.BuildTestNodeID([]byte{0x01}),
observedUptimes: map[ids.ID]uint32{constants.PrimaryNetworkID: 0},
}
updatedPeer1 := &peer{
id: ids.NodeID{0x01},
id: ids.BuildTestNodeID([]byte{0x01}),
observedUptimes: map[ids.ID]uint32{constants.PrimaryNetworkID: 1},
}
peer2 := &peer{
id: ids.NodeID{0x02},
id: ids.BuildTestNodeID([]byte{0x02}),
}
unknownPeer := &peer{
id: ids.NodeID{0xff},
id: ids.BuildTestNodeID([]byte{0xff}),
}
peer3 := &peer{
id: ids.NodeID{0x03},
id: ids.BuildTestNodeID([]byte{0x03}),
}
peer4 := &peer{
id: ids.NodeID{0x04},
id: ids.BuildTestNodeID([]byte{0x04}),
}

// add of first peer is handled
Expand Down Expand Up @@ -105,10 +105,10 @@ func TestSetSample(t *testing.T) {
set := NewSet()

peer1 := &peer{
id: ids.NodeID{0x01},
id: ids.BuildTestNodeID([]byte{0x01}),
}
peer2 := &peer{
id: ids.NodeID{0x02},
id: ids.BuildTestNodeID([]byte{0x02}),
}

// Case: Empty
Expand Down
8 changes: 4 additions & 4 deletions network/peer/upgrader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ func (t *tlsClientUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staki

func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeID, net.Conn, *staking.Certificate, error) {
if err := conn.Handshake(); err != nil {
return ids.NodeID{}, nil, nil, err
return ids.EmptyNodeID, nil, nil, err
}

state := conn.ConnectionState()
if len(state.PeerCertificates) == 0 {
return ids.NodeID{}, nil, nil, errNoCert
return ids.EmptyNodeID, nil, nil, errNoCert
}

tlsCert := state.PeerCertificates[0]
Expand All @@ -75,7 +75,7 @@ func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeI
peerCert, err := staking.ParseCertificate(tlsCert.Raw)
if err != nil {
invalidCerts.Inc()
return ids.NodeID{}, nil, nil, err
return ids.EmptyNodeID, nil, nil, err
}

// We validate the certificate here to attempt to make the validity of the
Expand All @@ -84,7 +84,7 @@ func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeI
// healthy.
if err := staking.ValidateCertificate(peerCert); err != nil {
invalidCerts.Inc()
return ids.NodeID{}, nil, nil, err
return ids.EmptyNodeID, nil, nil, err
}

nodeID := ids.NodeIDFromCert(peerCert)
Expand Down
2 changes: 1 addition & 1 deletion node/insecure_validator_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (i *insecureValidatorManager) Connected(vdrID ids.NodeID, nodeVersion *vers
// peer as a validator. Because each validator needs a txID associated
// with it, we hack one together by padding the nodeID with zeroes.
dummyTxID := ids.Empty
copy(dummyTxID[:], vdrID[:])
copy(dummyTxID[:], vdrID.Bytes())

err := i.vdrs.AddStaker(constants.PrimaryNetworkID, vdrID, nil, dummyTxID, i.weight)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func (n *Node) initNetworking() error {
// a validator. Because each validator needs a txID associated with it,
// we hack one together by just padding our nodeID with zeroes.
dummyTxID := ids.Empty
copy(dummyTxID[:], n.ID[:])
copy(dummyTxID[:], n.ID.Bytes())
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved

err := n.vdrs.AddStaker(
constants.PrimaryNetworkID,
Expand Down
10 changes: 5 additions & 5 deletions snow/consensus/snowman/poll/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ var (
blkID3 = ids.ID{3}
blkID4 = ids.ID{4}

vdr1 = ids.NodeID{1}
vdr2 = ids.NodeID{2}
vdr3 = ids.NodeID{3}
vdr4 = ids.NodeID{4}
vdr5 = ids.NodeID{5}
vdr1 = ids.BuildTestNodeID([]byte{0x01})
vdr2 = ids.BuildTestNodeID([]byte{0x02})
vdr3 = ids.BuildTestNodeID([]byte{0x03})
vdr4 = ids.BuildTestNodeID([]byte{0x04})
vdr5 = ids.BuildTestNodeID([]byte{0x05}) // k = 5
)

func TestNewSetErrorOnPollsMetrics(t *testing.T) {
Expand Down
8 changes: 3 additions & 5 deletions snow/engine/common/appsender/appsender_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ func (c *Client) SendAppRequest(ctx context.Context, nodeIDs set.Set[ids.NodeID]
nodeIDsBytes := make([][]byte, nodeIDs.Len())
i := 0
for nodeID := range nodeIDs {
nodeID := nodeID // Prevent overwrite in next iteration
nodeIDsBytes[i] = nodeID[:]
nodeIDsBytes[i] = nodeID.Bytes()
i++
}
_, err := c.client.SendAppRequest(
Expand All @@ -71,7 +70,7 @@ func (c *Client) SendAppResponse(ctx context.Context, nodeID ids.NodeID, request
_, err := c.client.SendAppResponse(
ctx,
&appsenderpb.SendAppResponseMsg{
NodeId: nodeID[:],
NodeId: nodeID.Bytes(),
RequestId: requestID,
Response: response,
},
Expand All @@ -93,8 +92,7 @@ func (c *Client) SendAppGossipSpecific(ctx context.Context, nodeIDs set.Set[ids.
nodeIDsBytes := make([][]byte, nodeIDs.Len())
i := 0
for nodeID := range nodeIDs {
nodeID := nodeID // Prevent overwrite in next iteration
nodeIDsBytes[i] = nodeID[:]
nodeIDsBytes[i] = nodeID.Bytes()
i++
}
_, err := c.client.SendAppGossipSpecific(
Expand Down
4 changes: 2 additions & 2 deletions snow/engine/common/requests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestRequests(t *testing.T) {
_, removed = req.Remove(ids.EmptyNodeID, 1)
require.False(removed)

_, removed = req.Remove(ids.NodeID{1}, 0)
_, removed = req.Remove(ids.BuildTestNodeID([]byte{0x01}), 0)
require.False(removed)

require.True(req.Contains(ids.Empty))
Expand All @@ -42,7 +42,7 @@ func TestRequests(t *testing.T) {
_, removed = req.Remove(ids.EmptyNodeID, 1)
require.False(removed)

_, removed = req.Remove(ids.NodeID{1}, 0)
_, removed = req.Remove(ids.BuildTestNodeID([]byte{0x01}), 0)
require.False(removed)

require.True(req.Contains(ids.Empty))
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/snowman/bootstrap/bootstrapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func TestBootstrapperUnknownByzantineResponse(t *testing.T) {
require.NoError(bs.Ancestors(context.Background(), peerID, *requestID+1, [][]byte{blkBytes1})) // respond with wrong request ID
require.Equal(oldReqID, *requestID)

require.NoError(bs.Ancestors(context.Background(), ids.NodeID{1, 2, 3}, *requestID, [][]byte{blkBytes1})) // respond from wrong peer
require.NoError(bs.Ancestors(context.Background(), ids.BuildTestNodeID([]byte{1, 2, 3}), *requestID, [][]byte{blkBytes1})) // respond from wrong peer
require.Equal(oldReqID, *requestID)

require.NoError(bs.Ancestors(context.Background(), peerID, *requestID, [][]byte{blkBytes0})) // respond with wrong block
Expand Down
4 changes: 2 additions & 2 deletions snow/networking/sender/sender_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func TestReliableMessages(t *testing.T) {

ctx := snow.DefaultConsensusContextTest()
vdrs := validators.NewManager()
require.NoError(vdrs.AddStaker(ctx.SubnetID, ids.NodeID{1}, nil, ids.Empty, 1))
require.NoError(vdrs.AddStaker(ctx.SubnetID, ids.BuildTestNodeID([]byte{1}), nil, ids.Empty, 1))
benchlist := benchlist.NewNoBenchlist()
tm, err := timeout.NewManager(
&timer.AdaptiveTimeoutConfig{
Expand Down Expand Up @@ -443,7 +443,7 @@ func TestReliableMessages(t *testing.T) {

go func() {
for i := 0; i < queriesToSend; i++ {
vdrIDs := set.Of(ids.NodeID{1})
vdrIDs := set.Of(ids.BuildTestNodeID([]byte{1}))

sender.SendPullQuery(context.Background(), vdrIDs, uint32(i), ids.Empty, 0)
time.Sleep(time.Duration(rand.Float64() * float64(time.Microsecond))) // #nosec G404
Expand Down
2 changes: 1 addition & 1 deletion snow/networking/timeout/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestManagerFire(t *testing.T) {
wg.Add(1)

manager.RegisterRequest(
ids.NodeID{},
ids.EmptyNodeID,
ids.ID{},
true,
ids.RequestID{},
Expand Down
4 changes: 2 additions & 2 deletions snow/networking/tracker/resource_tracker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func TestCPUTracker(t *testing.T) {
tracker, err := NewResourceTracker(prometheus.NewRegistry(), mockUser, meter.ContinuousFactory{}, time.Second)
require.NoError(err)

node1 := ids.NodeID{1}
node2 := ids.NodeID{2}
node1 := ids.BuildTestNodeID([]byte{1})
node2 := ids.BuildTestNodeID([]byte{2})

// Note that all the durations between start and end are [halflife].
startTime1 := time.Now()
Expand Down
4 changes: 2 additions & 2 deletions snow/networking/tracker/targeter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ func TestNewTargeter(t *testing.T) {
func TestTarget(t *testing.T) {
ctrl := gomock.NewController(t)

vdr := ids.NodeID{1}
vdr := ids.BuildTestNodeID([]byte{1})
vdrWeight := uint64(1)
totalVdrWeight := uint64(10)
nonVdr := ids.NodeID{2}
nonVdr := ids.BuildTestNodeID([]byte{2})
vdrs := validators.NewManager()
require.NoError(t, vdrs.AddStaker(constants.PrimaryNetworkID, vdr, nil, ids.Empty, 1))
require.NoError(t, vdrs.AddStaker(constants.PrimaryNetworkID, ids.GenerateTestNodeID(), nil, ids.Empty, totalVdrWeight-vdrWeight))
Expand Down
2 changes: 1 addition & 1 deletion snow/validators/gvalidators/validator_state_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (s *Server) GetValidatorSet(ctx context.Context, req *pb.GetValidatorSetReq
i := 0
for _, vdr := range vdrs {
vdrPB := &pb.Validator{
NodeId: vdr.NodeID[:],
NodeId: vdr.NodeID.Bytes(),
Weight: vdr.Weight,
}
if vdr.PublicKey != nil {
Expand Down
12 changes: 6 additions & 6 deletions snow/validators/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,9 @@ func TestGetMap(t *testing.T) {
func TestWeight(t *testing.T) {
require := require.New(t)

vdr0 := ids.NodeID{1}
vdr0 := ids.BuildTestNodeID([]byte{1})
weight0 := uint64(93)
vdr1 := ids.NodeID{2}
vdr1 := ids.BuildTestNodeID([]byte{2})
weight1 := uint64(123)

m := NewManager()
Expand Down Expand Up @@ -411,7 +411,7 @@ func TestString(t *testing.T) {
func TestAddCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
sk0, err := bls.NewSecretKey()
require.NoError(err)
pk0 := bls.PublicFromSecretKey(sk0)
Expand Down Expand Up @@ -442,7 +442,7 @@ func TestAddCallback(t *testing.T) {
func TestAddWeightCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(1)
weight1 := uint64(93)
Expand Down Expand Up @@ -480,7 +480,7 @@ func TestAddWeightCallback(t *testing.T) {
func TestRemoveWeightCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(93)
weight1 := uint64(92)
Expand Down Expand Up @@ -518,7 +518,7 @@ func TestRemoveWeightCallback(t *testing.T) {
func TestValidatorRemovedCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(93)

Expand Down
16 changes: 8 additions & 8 deletions snow/validators/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,9 @@ func TestSetMap(t *testing.T) {
func TestSetWeight(t *testing.T) {
require := require.New(t)

vdr0 := ids.NodeID{1}
vdr0 := ids.BuildTestNodeID([]byte{1})
weight0 := uint64(93)
vdr1 := ids.NodeID{2}
vdr1 := ids.BuildTestNodeID([]byte{2})
weight1 := uint64(123)

s := newSet()
Expand Down Expand Up @@ -332,10 +332,10 @@ func TestSetString(t *testing.T) {
require := require.New(t)

nodeID0 := ids.EmptyNodeID
nodeID1 := ids.NodeID{
nodeID1 := ids.BuildTestNodeID([]byte{
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
}
})

s := newSet()
require.NoError(s.Add(nodeID0, nil, ids.Empty, 1))
Expand Down Expand Up @@ -385,7 +385,7 @@ func (c *callbackListener) OnValidatorWeightChanged(nodeID ids.NodeID, oldWeight
func TestSetAddCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
sk0, err := bls.NewSecretKey()
require.NoError(err)
pk0 := bls.PublicFromSecretKey(sk0)
Expand Down Expand Up @@ -413,7 +413,7 @@ func TestSetAddCallback(t *testing.T) {
func TestSetAddWeightCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(1)
weight1 := uint64(93)
Expand Down Expand Up @@ -447,7 +447,7 @@ func TestSetAddWeightCallback(t *testing.T) {
func TestSetRemoveWeightCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(93)
weight1 := uint64(92)
Expand Down Expand Up @@ -481,7 +481,7 @@ func TestSetRemoveWeightCallback(t *testing.T) {
func TestSetValidatorRemovedCallback(t *testing.T) {
require := require.New(t)

nodeID0 := ids.NodeID{1}
nodeID0 := ids.BuildTestNodeID([]byte{1})
txID0 := ids.GenerateTestID()
weight0 := uint64(93)

Expand Down
6 changes: 3 additions & 3 deletions utils/beacon/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import (
func TestSet(t *testing.T) {
require := require.New(t)

id0 := ids.NodeID{0}
id1 := ids.NodeID{1}
id2 := ids.NodeID{2}
id0 := ids.BuildTestNodeID([]byte{0})
id1 := ids.BuildTestNodeID([]byte{1})
id2 := ids.BuildTestNodeID([]byte{2})

ip0 := ips.IPPort{
IP: net.IPv4zero,
Expand Down
8 changes: 4 additions & 4 deletions vms/platformvm/api/static_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

func TestBuildGenesisInvalidUTXOBalance(t *testing.T) {
require := require.New(t)
nodeID := ids.NodeID{1, 2, 3}
nodeID := ids.BuildTestNodeID([]byte{1, 2, 3})
addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
require.NoError(err)

Expand Down Expand Up @@ -62,7 +62,7 @@ func TestBuildGenesisInvalidUTXOBalance(t *testing.T) {

func TestBuildGenesisInvalidStakeWeight(t *testing.T) {
require := require.New(t)
nodeID := ids.NodeID{1, 2, 3}
nodeID := ids.BuildTestNodeID([]byte{1, 2, 3})
addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
require.NoError(err)

Expand Down Expand Up @@ -106,7 +106,7 @@ func TestBuildGenesisInvalidStakeWeight(t *testing.T) {

func TestBuildGenesisInvalidEndtime(t *testing.T) {
require := require.New(t)
nodeID := ids.NodeID{1, 2, 3}
nodeID := ids.BuildTestNodeID([]byte{1, 2, 3})
addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
require.NoError(err)

Expand Down Expand Up @@ -151,7 +151,7 @@ func TestBuildGenesisInvalidEndtime(t *testing.T) {

func TestBuildGenesisReturnsSortedValidators(t *testing.T) {
require := require.New(t)
nodeID := ids.NodeID{1}
nodeID := ids.BuildTestNodeID([]byte{1})
addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
require.NoError(err)

Expand Down
Loading
Loading