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.

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

Introduce ShortNodeID #3313

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
dce85fd
update
tsachiherman Aug 20, 2024
26073e7
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 21, 2024
05e180d
update
tsachiherman Aug 21, 2024
f87604d
update
tsachiherman Aug 21, 2024
71d596b
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 22, 2024
fbe8fc7
update
tsachiherman Aug 22, 2024
dab14e0
update
tsachiherman Aug 22, 2024
b44ca0d
fix unit tests
tsachiherman Aug 22, 2024
4f1ac5a
update
tsachiherman Aug 22, 2024
79cc15c
update
tsachiherman Aug 22, 2024
169d8bb
fix lint
tsachiherman Aug 22, 2024
7e5a753
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 22, 2024
4c5c396
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 22, 2024
fb4f52e
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 22, 2024
d4d2610
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 23, 2024
108bd24
fix merge errors
tsachiherman Aug 23, 2024
3d0e519
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 26, 2024
ced7eec
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 26, 2024
ff2dc60
update per CR
tsachiherman Aug 27, 2024
6e5692e
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 27, 2024
68845b4
rollback appsender_server changes.
tsachiherman Aug 27, 2024
111c0fe
fix unit test
tsachiherman Aug 27, 2024
0934976
fix unit tests
tsachiherman Aug 28, 2024
a952ad1
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Aug 28, 2024
181b8c6
update per CR
tsachiherman Sep 3, 2024
200fc37
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Sep 3, 2024
ae9598a
fix linter
tsachiherman Sep 3, 2024
f1b1a84
Merge branch 'master' into tsachi/acp20-nodeid
tsachiherman Sep 3, 2024
0af70b1
fix merge with master
tsachiherman Sep 3, 2024
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
2 changes: 1 addition & 1 deletion genesis/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (a Allocation) Compare(other Allocation) int {
}

type Staker struct {
NodeID ids.NodeID `json:"nodeID"`
NodeID ids.ShortNodeID `json:"nodeID"`
RewardAddress ids.ShortID `json:"rewardAddress"`
DelegationFee uint32 `json:"delegationFee"`
Signer *signer.ProofOfPossession `json:"signer,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion genesis/unparsed_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (ua UnparsedAllocation) Parse() (Allocation, error) {
}

type UnparsedStaker struct {
NodeID ids.NodeID `json:"nodeID"`
NodeID ids.ShortNodeID `json:"nodeID"`
RewardAddress string `json:"rewardAddress"`
DelegationFee uint32 `json:"delegationFee"`
Signer *signer.ProofOfPossession `json:"signer,omitempty"`
Expand Down
78 changes: 20 additions & 58 deletions ids/node_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,89 +4,51 @@
package ids

import (
"bytes"
"errors"
"fmt"

"github.com/ava-labs/avalanchego/staking"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/hashing"
)

const (
NodeIDPrefix = "NodeID-"
NodeIDLen = ShortIDLen
NodeIDLen = ShortIDLen // this would eventually be updated to 32 byte length.
)

var (
EmptyNodeID = NodeID{}

errShortNodeID = errors.New("insufficient NodeID length")

_ utils.Sortable[NodeID] = NodeID{}
)

type NodeID ShortID

func (id NodeID) String() string {
return ShortID(id).PrefixedString(NodeIDPrefix)
}

func (id NodeID) Bytes() []byte {
return id[:]
}
errWrongNodeIDLength = errors.New("wrong NodeID length")
)

func (id NodeID) MarshalJSON() ([]byte, error) {
return []byte(`"` + id.String() + `"`), nil
type NodeID struct {
ShortNodeID `serialize:"true"`
marun marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this diff is "forwards safe" I think we should be able to add an additional field (or somehow else change the serialized format of this struct) without breaking a ton of tests.

}

func (id NodeID) MarshalText() ([]byte, error) {
return []byte(id.String()), nil
func (id NodeID) Compare(other NodeID) int {
return id.ShortNodeID.Compare(other.ShortNodeID)
}

func (id *NodeID) UnmarshalJSON(b []byte) error {
str := string(b)
if str == nullStr { // If "null", do nothing
return nil
} else if len(str) <= 2+len(NodeIDPrefix) {
return fmt.Errorf("%w: expected to be > %d", errShortNodeID, 2+len(NodeIDPrefix))
}

lastIndex := len(str) - 1
if str[0] != '"' || str[lastIndex] != '"' {
return errMissingQuotes
// NodeIDFromString is the inverse of NodeID.String()
func NodeIDFromString(nodeIDStr string) (NodeID, error) {
asShort, err := ShortFromPrefixedString(nodeIDStr, NodeIDPrefix)
if err != nil {
return NodeID{}, err
}

var err error
*id, err = NodeIDFromString(str[1:lastIndex])
return err
}

func (id *NodeID) UnmarshalText(text []byte) error {
return id.UnmarshalJSON(text)
}

func (id NodeID) Compare(other NodeID) int {
return bytes.Compare(id[:], other[:])
return NodeID{ShortNodeID: ShortNodeID(asShort)}, nil
}

// ToNodeID attempt to convert a byte slice into a node id
func ToNodeID(bytes []byte) (NodeID, error) {
nodeID, err := ToShortID(bytes)
return NodeID(nodeID), err
return NodeID{ShortNodeID: ShortNodeID(nodeID)}, err
}

func NodeIDFromCert(cert *staking.Certificate) NodeID {
return hashing.ComputeHash160Array(
hashing.ComputeHash256(cert.Raw),
)
}

// NodeIDFromString is the inverse of NodeID.String()
func NodeIDFromString(nodeIDStr string) (NodeID, error) {
asShort, err := ShortFromPrefixedString(nodeIDStr, NodeIDPrefix)
if err != nil {
return NodeID{}, err
func ParseNodeID(bytes []byte) (NodeID, error) {
if len(bytes) == ShortIDLen {
var node NodeID
copy(node.ShortNodeID[:], bytes)
return node, nil
}
return NodeID(asShort), nil
return NodeID{}, errWrongNodeIDLength
}
28 changes: 14 additions & 14 deletions ids/node_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import (
func TestNodeIDEquality(t *testing.T) {
require := require.New(t)

id := NodeID{24}
idCopy := NodeID{24}
id := NodeID{ShortNodeID: ShortNodeID{24}}
idCopy := NodeID{ShortNodeID: ShortNodeID{24}}
require.Equal(id, idCopy)
id2 := NodeID{}
require.NotEqual(id, id2)
Expand All @@ -26,7 +26,7 @@ func TestNodeIDEquality(t *testing.T) {
func TestNodeIDFromString(t *testing.T) {
require := require.New(t)

id := NodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}
id := NodeID{ShortNodeID: ShortNodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}}
idStr := id.String()
id2, err := NodeIDFromString(idStr)
require.NoError(err)
Expand Down Expand Up @@ -76,7 +76,7 @@ func TestNodeIDMarshalJSON(t *testing.T) {
},
{
`ID("ava labs")`,
NodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'},
NodeID{ShortNodeID: ShortNodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}},
[]byte(`"NodeID-9tLMkeWFhWXd8QZc4rSiS5meuVXF5kRsz"`),
nil,
},
Expand Down Expand Up @@ -108,7 +108,7 @@ func TestNodeIDUnmarshalJSON(t *testing.T) {
{
`NodeID("ava labs")`,
[]byte(`"NodeID-9tLMkeWFhWXd8QZc4rSiS5meuVXF5kRsz"`),
NodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'},
NodeID{ShortNodeID: ShortNodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}},
nil,
},
{
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestNodeIDString(t *testing.T) {
expected string
}{
{"NodeID{}", NodeID{}, "NodeID-111111111111111111116DBWJs"},
{"NodeID{24}", NodeID{24}, "NodeID-3BuDc2d1Efme5Apba6SJ8w3Tz7qeh6mHt"},
{"NodeID{24}", NodeID{ShortNodeID: ShortNodeID{24}}, "NodeID-3BuDc2d1Efme5Apba6SJ8w3Tz7qeh6mHt"},
}
for _, tt := range tests {
t.Run(tt.label, func(t *testing.T) {
Expand All @@ -174,8 +174,8 @@ func TestNodeIDMapMarshalling(t *testing.T) {
require := require.New(t)

originalMap := map[NodeID]int{
{'e', 'v', 'a', ' ', 'l', 'a', 'b', 's'}: 1,
{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}: 2,
{ShortNodeID: ShortNodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}}: 1,
{ShortNodeID: ShortNodeID{'a', 'v', 'a', ' ', 'l', 'a', 'b', 's'}}: 2,
}
mapJSON, err := json.Marshal(originalMap)
require.NoError(err)
Expand All @@ -192,18 +192,18 @@ func TestNodeIDCompare(t *testing.T) {
expected int
}{
{
a: NodeID{1},
b: NodeID{0},
a: NodeID{ShortNodeID: ShortNodeID{1}},
b: NodeID{ShortNodeID: ShortNodeID{0}},
expected: 1,
},
{
a: NodeID{1},
b: NodeID{1},
a: NodeID{ShortNodeID: ShortNodeID{1}},
b: NodeID{ShortNodeID: ShortNodeID{1}},
expected: 0,
},
{
a: NodeID{1, 0},
b: NodeID{1, 2},
a: NodeID{ShortNodeID: ShortNodeID{1, 0}},
b: NodeID{ShortNodeID: ShortNodeID{1, 2}},
expected: -1,
},
}
Expand Down
104 changes: 104 additions & 0 deletions ids/short_node_id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package ids

import (
"bytes"
"errors"
"fmt"

"github.com/ava-labs/avalanchego/staking"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/hashing"
)

const ShortNodeIDLen = ShortIDLen

var (
EmptyShortNodeID = ShortNodeID{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've kind of grown to hate this Empty... pattern. Feels like we should probably just use the zero value.

Not asking for anything to change in this PR to be clear... Just felt like voicing that.


errShortNodeID = errors.New("insufficient ShortNodeID length")

_ utils.Sortable[ShortNodeID] = ShortNodeID{}
)

type ShortNodeID ShortID

func (sn ShortNodeID) Bytes() []byte {
return sn[:]
}

func ToShortNodeID(bytes []byte) (ShortNodeID, error) {
nodeID, err := ToShortID(bytes)
return ShortNodeID(nodeID), err
}

func (sn ShortNodeID) String() string {
return ShortID(sn).PrefixedString(NodeIDPrefix)
}

// ShortNodeIDFromString is the inverse of ShortNodeID.String()
func ShortNodeIDFromString(nodeIDStr string) (ShortNodeID, error) {
asShort, err := ShortFromPrefixedString(nodeIDStr, NodeIDPrefix)
if err != nil {
return EmptyShortNodeID, err
}
return ShortNodeID(asShort), nil
}

// ShortNodeIDFromNodeID is the inverse of NodeID.String()
func ShortNodeIDFromNodeID(nodeID NodeID) (ShortNodeID, error) {
if nodeID == EmptyNodeID {
return EmptyShortNodeID, nil
}
res, err := ToShortNodeID(nodeID.Bytes())
if err != nil {
return EmptyShortNodeID, fmt.Errorf("failed converting NodeID to ShortNodeID, %w", err)
}
return res, nil
}

func (sn ShortNodeID) MarshalJSON() ([]byte, error) {
return []byte(`"` + sn.String() + `"`), nil
}

func (sn *ShortNodeID) UnmarshalJSON(b []byte) error {
str := string(b)
if str == nullStr { // If "null", do nothing
return nil
} else if len(str) <= 2+len(NodeIDPrefix) {
return fmt.Errorf("%w: expected to be > %d", errShortNodeID, 2+len(NodeIDPrefix))
}

lastIndex := len(str) - 1
if str[0] != '"' || str[lastIndex] != '"' {
return errMissingQuotes
}

var err error
*sn, err = ShortNodeIDFromString(str[1:lastIndex])
return err
}

func (sn ShortNodeID) MarshalText() ([]byte, error) {
return []byte(sn.String()), nil
}

func (sn *ShortNodeID) UnmarshalText(text []byte) error {
return sn.UnmarshalJSON(text)
}

func (sn ShortNodeID) Compare(other ShortNodeID) int {
return bytes.Compare(sn[:], other[:])
}

func (sn ShortNodeID) NodeID() NodeID {
return NodeID{ShortNodeID: sn}
}

func ShortNodeIDFromCert(cert *staking.Certificate) ShortNodeID {
return hashing.ComputeHash160Array(
hashing.ComputeHash256(cert.Raw),
)
}
Loading
Loading