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

Pass the nodeID downstream of acp118 verifier #3606

Closed
wants to merge 10 commits into from
Closed

Pass the nodeID downstream of acp118 verifier #3606

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2ce94f4
update
tsachiherman Dec 17, 2024
39e29e3
update interface
tsachiherman Dec 17, 2024
030fd30
update
tsachiherman Dec 17, 2024
7694421
update
tsachiherman Dec 17, 2024
1668c78
Merge branch 'master' into tsachi/acp118-nodeid
tsachiherman Dec 17, 2024
643db30
reorder
tsachiherman Dec 18, 2024
259f773
update
tsachiherman Dec 18, 2024
172b466
Merge branch 'master' into tsachi/acp118-nodeid
tsachiherman Dec 18, 2024
ad80975
update unit test per feedback.
tsachiherman Dec 18, 2024
c3494e8
update
tsachiherman Dec 18, 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
5 changes: 3 additions & 2 deletions network/p2p/acp118/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Verifier interface {
ctx context.Context,
message *warp.UnsignedMessage,
justification []byte,
nodeID ids.NodeID,
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
) *common.AppError
}

Expand Down Expand Up @@ -65,7 +66,7 @@ type Handler struct {

func (h *Handler) AppRequest(
ctx context.Context,
_ ids.NodeID,
nodeID ids.NodeID,
_ time.Time,
requestBytes []byte,
) ([]byte, *common.AppError) {
Expand All @@ -90,7 +91,7 @@ func (h *Handler) AppRequest(
return signatureToResponse(signatureBytes)
}

if err := h.verifier.Verify(ctx, msg, request.Justification); err != nil {
if err := h.verifier.Verify(ctx, msg, request.Justification, nodeID); err != nil {
return nil, err
}

Expand Down
56 changes: 34 additions & 22 deletions network/p2p/acp118/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,23 @@ func TestHandler(t *testing.T) {
tests := []struct {
name string
cacher cache.Cacher[ids.ID, []byte]
verifier Verifier
verifierErrs []*common.AppError
expectedErrs []error
}{
{
name: "signature fails verification",
cacher: &cache.Empty[ids.ID, []byte]{},
verifier: &testVerifier{
Errs: []*common.AppError{
{Code: 123},
},
verifierErrs: []*common.AppError{
{Code: 123},
},
expectedErrs: []error{
&common.AppError{Code: 123},
},
},
{
name: "signature signed",
cacher: &cache.Empty[ids.ID, []byte]{},
verifier: &testVerifier{},
name: "signature signed",
cacher: &cache.Empty[ids.ID, []byte]{},
verifierErrs: nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be deleted entirely iirc because the zero value of a slice is safe to use

expectedErrs: []error{
nil,
},
Expand All @@ -54,11 +52,9 @@ func TestHandler(t *testing.T) {
cacher: &cache.LRU[ids.ID, []byte]{
Size: 1,
},
verifier: &testVerifier{
Errs: []*common.AppError{
nil,
{Code: 123}, // The valid response should be cached
},
verifierErrs: []*common.AppError{
nil,
{Code: 123}, // The valid response should be cached
},
expectedErrs: []error{
nil,
Expand All @@ -78,9 +74,10 @@ func TestHandler(t *testing.T) {
networkID := uint32(123)
chainID := ids.GenerateTestID()
signer := warp.NewSigner(sk, networkID, chainID)
h := NewCachedHandler(tt.cacher, tt.verifier, signer)
clientNodeID := ids.GenerateTestNodeID()
serverNodeID := ids.GenerateTestNodeID()
h := NewCachedHandler(tt.cacher, newTestVerifier(t, clientNodeID, tt.verifierErrs), signer)

c := p2ptest.NewClient(
t,
ctx,
Expand Down Expand Up @@ -108,7 +105,7 @@ func TestHandler(t *testing.T) {
expectedErr error
handled = make(chan struct{})
)
onResponse := func(_ context.Context, _ ids.NodeID, responseBytes []byte, appErr error) {
onResponse := func(_ context.Context, nodeID ids.NodeID, responseBytes []byte, appErr error) {
defer func() {
handled <- struct{}{}
}()
Expand All @@ -131,6 +128,8 @@ func TestHandler(t *testing.T) {
if ok {
require.Equal(sig, response.Signature)
}

require.Equal(serverNodeID, nodeID)
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
}

for _, expectedErr = range tt.expectedErrs {
Expand All @@ -143,18 +142,31 @@ func TestHandler(t *testing.T) {

// The zero value of testVerifier allows signing
type testVerifier struct {
Errs []*common.AppError
t *testing.T
errs []*common.AppError
clientNodeID ids.NodeID
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
}

func newTestVerifier(t *testing.T, clientNodeID ids.NodeID, errs []*common.AppError) *testVerifier {
return &testVerifier{
t: t,
errs: errs,
clientNodeID: clientNodeID,
}
}
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved

func (t *testVerifier) Verify(
context.Context,
*warp.UnsignedMessage,
[]byte,
_ context.Context,
_ *warp.UnsignedMessage,
justification []byte,
nodeID ids.NodeID,
) *common.AppError {
if len(t.Errs) == 0 {
require.Equal(t.t, t.clientNodeID, nodeID)
require.Equal(t.t, []byte("justification"), justification)
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
if len(t.errs) == 0 {
return nil
}
err := t.Errs[0]
t.Errs = t.Errs[1:]
err := t.errs[0]
t.errs = t.errs[1:]
return err
}
3 changes: 2 additions & 1 deletion vms/example/xsvm/warp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package xsvm
import (
"context"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/network/p2p/acp118"
"github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/vms/platformvm/warp"
Expand All @@ -16,6 +17,6 @@ var _ acp118.Verifier = (*acp118Verifier)(nil)
// acp118Verifier allows signing all warp messages
type acp118Verifier struct{}

func (acp118Verifier) Verify(context.Context, *warp.UnsignedMessage, []byte) *common.AppError {
func (acp118Verifier) Verify(context.Context, *warp.UnsignedMessage, []byte, ids.NodeID) *common.AppError {
return nil
}
1 change: 1 addition & 0 deletions vms/platformvm/network/warp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func (s signatureRequestVerifier) Verify(
_ context.Context,
unsignedMessage *warp.UnsignedMessage,
justification []byte,
_ ids.NodeID,
) *common.AppError {
msg, err := payload.ParseAddressedCall(unsignedMessage.Payload)
if err != nil {
Expand Down
Loading