From c1d6b8c223acf2b6869ca359440657fb6f8e0230 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Thu, 22 Aug 2024 20:44:15 +0000 Subject: [PATCH 1/5] extract interface --- peers/app_request_network.go | 42 ++++++++++++++----- relayer/application_relayer.go | 4 +- relayer/main/main.go | 4 +- relayer/network_utils.go | 6 +-- signature-aggregator/aggregator/aggregator.go | 4 +- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/peers/app_request_network.go b/peers/app_request_network.go index 512a32a8..d26810e6 100644 --- a/peers/app_request_network.go +++ b/peers/app_request_network.go @@ -28,7 +28,27 @@ const ( DefaultAppRequestTimeout = time.Second * 2 ) -type AppRequestNetwork struct { +type AppRequestNetwork interface { + ConnectPeers(nodeIDs set.Set[ids.NodeID]) set.Set[ids.NodeID] + ConnectToCanonicalValidators(subnetID ids.ID) ( + *ConnectedCanonicalValidators, + error, + ) + GetSubnetID(blockchainID ids.ID) (ids.ID, error) + RegisterAppRequest(requestID ids.RequestID) + RegisterRequestID( + requestID uint32, + numExpectedResponse int, + ) chan message.InboundMessage + Send( + msg message.OutboundMessage, + nodeIDs set.Set[ids.NodeID], + subnetID ids.ID, + allower subnets.Allower, + ) set.Set[ids.NodeID] +} + +type appRequestNetwork struct { network network.Network handler *RelayerExternalHandler infoAPI *InfoAPI @@ -44,7 +64,7 @@ func NewNetwork( registerer prometheus.Registerer, trackedSubnets set.Set[ids.ID], cfg Config, -) (*AppRequestNetwork, error) { +) (AppRequestNetwork, error) { logger := logging.NewLogger( "p2p-network", logging.NewWrappedCore( @@ -98,7 +118,7 @@ func NewNetwork( validatorClient := validators.NewCanonicalValidatorClient(logger, cfg.GetPChainAPI()) - arNetwork := &AppRequestNetwork{ + arNetwork := &appRequestNetwork{ network: testNetwork, handler: handler, infoAPI: infoAPI, @@ -116,7 +136,7 @@ func NewNetwork( // ConnectPeers connects the network to peers with the given nodeIDs. // Returns the set of nodeIDs that were successfully connected to. -func (n *AppRequestNetwork) ConnectPeers(nodeIDs set.Set[ids.NodeID]) set.Set[ids.NodeID] { +func (n *appRequestNetwork) ConnectPeers(nodeIDs set.Set[ids.NodeID]) set.Set[ids.NodeID] { n.lock.Lock() defer n.lock.Unlock() @@ -200,7 +220,7 @@ func (c *ConnectedCanonicalValidators) GetValidator(nodeID ids.NodeID) (*warp.Va // ConnectToCanonicalValidators connects to the canonical validators of the given subnet and returns the connected // validator information -func (n *AppRequestNetwork) ConnectToCanonicalValidators(subnetID ids.ID) (*ConnectedCanonicalValidators, error) { +func (n *appRequestNetwork) ConnectToCanonicalValidators(subnetID ids.ID) (*ConnectedCanonicalValidators, error) { // Get the subnet's current canonical validator set startPChainAPICall := time.Now() validatorSet, totalValidatorWeight, err := n.validatorClient.GetCurrentCanonicalValidatorSet(subnetID) @@ -240,7 +260,7 @@ func (n *AppRequestNetwork) ConnectToCanonicalValidators(subnetID ids.ID) (*Conn }, nil } -func (n *AppRequestNetwork) Send( +func (n *appRequestNetwork) Send( msg message.OutboundMessage, nodeIDs set.Set[ids.NodeID], subnetID ids.ID, @@ -249,13 +269,13 @@ func (n *AppRequestNetwork) Send( return n.network.Send(msg, avagoCommon.SendConfig{NodeIDs: nodeIDs}, subnetID, allower) } -func (n *AppRequestNetwork) RegisterAppRequest(requestID ids.RequestID) { +func (n *appRequestNetwork) RegisterAppRequest(requestID ids.RequestID) { n.handler.RegisterAppRequest(requestID) } -func (n *AppRequestNetwork) RegisterRequestID(requestID uint32, numExpectedResponse int) chan message.InboundMessage { +func (n *appRequestNetwork) RegisterRequestID(requestID uint32, numExpectedResponse int) chan message.InboundMessage { return n.handler.RegisterRequestID(requestID, numExpectedResponse) } -func (n *AppRequestNetwork) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { +func (n *appRequestNetwork) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { return n.validatorClient.GetSubnetID(context.Background(), blockchainID) } @@ -263,10 +283,10 @@ func (n *AppRequestNetwork) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { // Metrics // -func (n *AppRequestNetwork) setInfoAPICallLatencyMS(latency float64) { +func (n *appRequestNetwork) setInfoAPICallLatencyMS(latency float64) { n.metrics.infoAPICallLatencyMS.Observe(latency) } -func (n *AppRequestNetwork) setPChainAPICallLatencyMS(latency float64) { +func (n *appRequestNetwork) setPChainAPICallLatencyMS(latency float64) { n.metrics.pChainAPICallLatencyMS.Observe(latency) } diff --git a/relayer/application_relayer.go b/relayer/application_relayer.go index bb0c2f3e..89a33194 100644 --- a/relayer/application_relayer.go +++ b/relayer/application_relayer.go @@ -57,7 +57,7 @@ type CheckpointManager interface { type ApplicationRelayer struct { logger logging.Logger metrics *ApplicationRelayerMetrics - network *peers.AppRequestNetwork + network peers.AppRequestNetwork sourceBlockchain config.SourceBlockchain signingSubnetID ids.ID destinationClient vms.DestinationClient @@ -71,7 +71,7 @@ type ApplicationRelayer struct { func NewApplicationRelayer( logger logging.Logger, metrics *ApplicationRelayerMetrics, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, relayerID database.RelayerID, destinationClient vms.DestinationClient, sourceBlockchain config.SourceBlockchain, diff --git a/relayer/main/main.go b/relayer/main/main.go index 5340bba2..b3b72f96 100644 --- a/relayer/main/main.go +++ b/relayer/main/main.go @@ -364,7 +364,7 @@ func createApplicationRelayers( relayerMetrics *relayer.ApplicationRelayerMetrics, db database.RelayerDatabase, ticker *utils.Ticker, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, cfg *config.Config, sourceClients map[ids.ID]ethclient.Client, destinationClients map[ids.ID]vms.DestinationClient, @@ -423,7 +423,7 @@ func createApplicationRelayersForSourceChain( db database.RelayerDatabase, ticker *utils.Ticker, sourceBlockchain config.SourceBlockchain, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, cfg *config.Config, currentHeight uint64, destinationClients map[ids.ID]vms.DestinationClient, diff --git a/relayer/network_utils.go b/relayer/network_utils.go index 4931ee6a..fc1e925a 100644 --- a/relayer/network_utils.go +++ b/relayer/network_utils.go @@ -26,7 +26,7 @@ import ( // or if the subnet supports all destinations, by the quora of all configured destinations. func InitializeConnectionsAndCheckStake( logger logging.Logger, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, cfg *config.Config, ) error { for _, sourceBlockchain := range cfg.SourceBlockchains { @@ -53,7 +53,7 @@ func InitializeConnectionsAndCheckStake( // verify that we have connected to a threshold of stake. func connectToNonPrimaryNetworkPeers( logger logging.Logger, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, cfg *config.Config, sourceBlockchain *config.SourceBlockchain, ) error { @@ -87,7 +87,7 @@ func connectToNonPrimaryNetworkPeers( // to a threshold of stake for each blockchain. func connectToPrimaryNetworkPeers( logger logging.Logger, - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, cfg *config.Config, sourceBlockchain *config.SourceBlockchain, ) error { diff --git a/signature-aggregator/aggregator/aggregator.go b/signature-aggregator/aggregator/aggregator.go index 7a0f441d..979764a0 100644 --- a/signature-aggregator/aggregator/aggregator.go +++ b/signature-aggregator/aggregator/aggregator.go @@ -51,7 +51,7 @@ var ( ) type SignatureAggregator struct { - network *peers.AppRequestNetwork + network peers.AppRequestNetwork // protected by subnetsMapLock subnetIDsByBlockchainID map[ids.ID]ids.ID logger logging.Logger @@ -63,7 +63,7 @@ type SignatureAggregator struct { } func NewSignatureAggregator( - network *peers.AppRequestNetwork, + network peers.AppRequestNetwork, logger logging.Logger, signatureCacheSize uint64, metrics *metrics.SignatureAggregatorMetrics, From e586c9288413588f663397547bc36ca83c48da08 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Thu, 22 Aug 2024 17:48:31 +0000 Subject: [PATCH 2/5] add AppReqNet mocks and a few initial tests --- .github/workflows/mock_checker.yml | 35 +++++ peers/app_request_network.go | 2 + peers/mocks/mock_app_request_network.go | 128 ++++++++++++++++++ scripts/build.sh | 3 + scripts/generate.sh | 15 ++ signature-aggregator/aggregator/aggregator.go | 4 + .../aggregator/aggregator_test.go | 87 ++++++++++++ 7 files changed, 274 insertions(+) create mode 100644 .github/workflows/mock_checker.yml create mode 100644 peers/mocks/mock_app_request_network.go create mode 100755 scripts/generate.sh create mode 100644 signature-aggregator/aggregator/aggregator_test.go diff --git a/.github/workflows/mock_checker.yml b/.github/workflows/mock_checker.yml new file mode 100644 index 00000000..0454e1d4 --- /dev/null +++ b/.github/workflows/mock_checker.yml @@ -0,0 +1,35 @@ +name: Check generated code is up to date + +on: + push: + branches: + - main + pull_request: + branches: + - "**" + +jobs: + generated_code: + name: generated_code + runs-on: ubuntu-22.04 + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: "go.mod" + + - name: Generate code + run: | + scripts/generate.sh + + - name: Print diff + run: git --no-pager diff + + - name: Fail if diff exists + run: git --no-pager diff --quiet diff --git a/peers/app_request_network.go b/peers/app_request_network.go index d26810e6..eed93bfb 100644 --- a/peers/app_request_network.go +++ b/peers/app_request_network.go @@ -1,6 +1,8 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. +//go:generate mockgen -source=$GOFILE -destination=./mocks/mock_app_request_network.go -package=mocks + package peers import ( diff --git a/peers/mocks/mock_app_request_network.go b/peers/mocks/mock_app_request_network.go new file mode 100644 index 00000000..f989358c --- /dev/null +++ b/peers/mocks/mock_app_request_network.go @@ -0,0 +1,128 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: app_request_network.go +// +// Generated by this command: +// +// mockgen -source=app_request_network.go -destination=./mocks/mock_app_request_network.go -package=mocks +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + ids "github.com/ava-labs/avalanchego/ids" + message "github.com/ava-labs/avalanchego/message" + subnets "github.com/ava-labs/avalanchego/subnets" + set "github.com/ava-labs/avalanchego/utils/set" + peers "github.com/ava-labs/awm-relayer/peers" + gomock "go.uber.org/mock/gomock" +) + +// MockAppRequestNetwork is a mock of AppRequestNetwork interface. +type MockAppRequestNetwork struct { + ctrl *gomock.Controller + recorder *MockAppRequestNetworkMockRecorder +} + +// MockAppRequestNetworkMockRecorder is the mock recorder for MockAppRequestNetwork. +type MockAppRequestNetworkMockRecorder struct { + mock *MockAppRequestNetwork +} + +// NewMockAppRequestNetwork creates a new mock instance. +func NewMockAppRequestNetwork(ctrl *gomock.Controller) *MockAppRequestNetwork { + mock := &MockAppRequestNetwork{ctrl: ctrl} + mock.recorder = &MockAppRequestNetworkMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAppRequestNetwork) EXPECT() *MockAppRequestNetworkMockRecorder { + return m.recorder +} + +// ConnectPeers mocks base method. +func (m *MockAppRequestNetwork) ConnectPeers(nodeIDs set.Set[ids.NodeID]) set.Set[ids.NodeID] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectPeers", nodeIDs) + ret0, _ := ret[0].(set.Set[ids.NodeID]) + return ret0 +} + +// ConnectPeers indicates an expected call of ConnectPeers. +func (mr *MockAppRequestNetworkMockRecorder) ConnectPeers(nodeIDs any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectPeers", reflect.TypeOf((*MockAppRequestNetwork)(nil).ConnectPeers), nodeIDs) +} + +// ConnectToCanonicalValidators mocks base method. +func (m *MockAppRequestNetwork) ConnectToCanonicalValidators(subnetID ids.ID) (*peers.ConnectedCanonicalValidators, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectToCanonicalValidators", subnetID) + ret0, _ := ret[0].(*peers.ConnectedCanonicalValidators) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConnectToCanonicalValidators indicates an expected call of ConnectToCanonicalValidators. +func (mr *MockAppRequestNetworkMockRecorder) ConnectToCanonicalValidators(subnetID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectToCanonicalValidators", reflect.TypeOf((*MockAppRequestNetwork)(nil).ConnectToCanonicalValidators), subnetID) +} + +// GetSubnetID mocks base method. +func (m *MockAppRequestNetwork) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSubnetID", blockchainID) + ret0, _ := ret[0].(ids.ID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSubnetID indicates an expected call of GetSubnetID. +func (mr *MockAppRequestNetworkMockRecorder) GetSubnetID(blockchainID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetID", reflect.TypeOf((*MockAppRequestNetwork)(nil).GetSubnetID), blockchainID) +} + +// RegisterAppRequest mocks base method. +func (m *MockAppRequestNetwork) RegisterAppRequest(requestID ids.RequestID) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RegisterAppRequest", requestID) +} + +// RegisterAppRequest indicates an expected call of RegisterAppRequest. +func (mr *MockAppRequestNetworkMockRecorder) RegisterAppRequest(requestID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterAppRequest", reflect.TypeOf((*MockAppRequestNetwork)(nil).RegisterAppRequest), requestID) +} + +// RegisterRequestID mocks base method. +func (m *MockAppRequestNetwork) RegisterRequestID(requestID uint32, numExpectedResponse int) chan message.InboundMessage { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RegisterRequestID", requestID, numExpectedResponse) + ret0, _ := ret[0].(chan message.InboundMessage) + return ret0 +} + +// RegisterRequestID indicates an expected call of RegisterRequestID. +func (mr *MockAppRequestNetworkMockRecorder) RegisterRequestID(requestID, numExpectedResponse any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterRequestID", reflect.TypeOf((*MockAppRequestNetwork)(nil).RegisterRequestID), requestID, numExpectedResponse) +} + +// Send mocks base method. +func (m *MockAppRequestNetwork) Send(msg message.OutboundMessage, nodeIDs set.Set[ids.NodeID], subnetID ids.ID, allower subnets.Allower) set.Set[ids.NodeID] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Send", msg, nodeIDs, subnetID, allower) + ret0, _ := ret[0].(set.Set[ids.NodeID]) + return ret0 +} + +// Send indicates an expected call of Send. +func (mr *MockAppRequestNetworkMockRecorder) Send(msg, nodeIDs, subnetID, allower any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Send", reflect.TypeOf((*MockAppRequestNetwork)(nil).Send), msg, nodeIDs, subnetID, allower) +} diff --git a/scripts/build.sh b/scripts/build.sh index 3acd64a9..a25a2ed6 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -2,11 +2,14 @@ # Copyright (C) 2024, Ava Labs, Inc. All rights reserved. # See the file LICENSE for licensing terms. +set -e errexit + # Root directory root=$( cd "$(dirname "${BASH_SOURCE[0]}")" cd .. && pwd ) +"$root"/scripts/generate.sh "$root"/scripts/build_relayer.sh "$root"/scripts/build_signature_aggregator.sh diff --git a/scripts/generate.sh b/scripts/generate.sh new file mode 100755 index 00000000..1193ec39 --- /dev/null +++ b/scripts/generate.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# Copyright (C) 2024, Ava Labs, Inc. All rights reserved. +# See the file LICENSE for licensing terms. + +set -e errexit + +# Root directory +root=$( + cd "$(dirname "${BASH_SOURCE[0]}")" + cd .. && pwd +) + +source "$root"/scripts/versions.sh +go install -v "go.uber.org/mock/mockgen@$(getDepVersion go.uber.org/mock)" +PATH="$PATH:$(go env GOPATH)/bin" go generate ./... diff --git a/signature-aggregator/aggregator/aggregator.go b/signature-aggregator/aggregator/aggregator.go index 979764a0..161066fc 100644 --- a/signature-aggregator/aggregator/aggregator.go +++ b/signature-aggregator/aggregator/aggregator.go @@ -343,6 +343,8 @@ func (s *SignatureAggregator) CreateSignedMessage( return nil, errNotEnoughSignatures } +// TODO: consider making this function private. its only reference seems to be +// within this module. func (s *SignatureAggregator) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { s.subnetsMapLock.RLock() subnetID, ok := s.subnetIDsByBlockchainID[blockchainID] @@ -359,6 +361,8 @@ func (s *SignatureAggregator) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { return subnetID, nil } +// TODO: consider making this function private. its only reference seems to be +// within this module. func (s *SignatureAggregator) SetSubnetID(blockchainID ids.ID, subnetID ids.ID) { s.subnetsMapLock.Lock() s.subnetIDsByBlockchainID[blockchainID] = subnetID diff --git a/signature-aggregator/aggregator/aggregator_test.go b/signature-aggregator/aggregator/aggregator_test.go new file mode 100644 index 00000000..13a6952c --- /dev/null +++ b/signature-aggregator/aggregator/aggregator_test.go @@ -0,0 +1,87 @@ +package aggregator + +import ( + "testing" + + "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/vms/platformvm/warp" + "github.com/ava-labs/awm-relayer/peers" + "github.com/ava-labs/awm-relayer/peers/mocks" + "github.com/ava-labs/awm-relayer/signature-aggregator/metrics" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +var sigAggMetrics *metrics.SignatureAggregatorMetrics +var messageCreator message.Creator + +func instantiateAggregator(t *testing.T) ( + *SignatureAggregator, + *mocks.MockAppRequestNetwork, +) { + mockNetwork := mocks.NewMockAppRequestNetwork(gomock.NewController(t)) + if sigAggMetrics == nil { + sigAggMetrics = metrics.NewSignatureAggregatorMetrics(prometheus.DefaultRegisterer) + } + if messageCreator == nil { + var err error + messageCreator, err = message.NewCreator( + logging.NoLog{}, + prometheus.DefaultRegisterer, + constants.DefaultNetworkCompressionType, + constants.DefaultNetworkMaximumInboundTimeout, + ) + require.Equal(t, err, nil) + } + aggregator, err := NewSignatureAggregator( + mockNetwork, + logging.NoLog{}, + 1024, + sigAggMetrics, + messageCreator, + ) + require.Equal(t, err, nil) + return aggregator, mockNetwork +} + +func TestCreateSignedMessageFailsWithNoValidators(t *testing.T) { + aggregator, mockNetwork := instantiateAggregator(t) + msg, err := warp.NewUnsignedMessage(0, ids.Empty, []byte{}) + require.Equal(t, err, nil) + mockNetwork.EXPECT().GetSubnetID(ids.Empty).Return(ids.Empty, nil) + mockNetwork.EXPECT().ConnectToCanonicalValidators(ids.Empty).Return( + &peers.ConnectedCanonicalValidators{ + ConnectedWeight: 0, + TotalValidatorWeight: 0, + ValidatorSet: []*warp.Validator{}, + }, + nil, + ) + _, err = aggregator.CreateSignedMessage(msg, ids.Empty, 80) + require.ErrorContains(t, err, "no signatures") +} + +func TestCreateSignedMessageFailsWithoutSufficientConnectedStake(t *testing.T) { + aggregator, mockNetwork := instantiateAggregator(t) + msg, err := warp.NewUnsignedMessage(0, ids.Empty, []byte{}) + require.Equal(t, err, nil) + mockNetwork.EXPECT().GetSubnetID(ids.Empty).Return(ids.Empty, nil) + mockNetwork.EXPECT().ConnectToCanonicalValidators(ids.Empty).Return( + &peers.ConnectedCanonicalValidators{ + ConnectedWeight: 0, + TotalValidatorWeight: 1, + ValidatorSet: []*warp.Validator{}, + }, + nil, + ) + _, err = aggregator.CreateSignedMessage(msg, ids.Empty, 80) + require.ErrorContains( + t, + err, + "failed to connect to a threshold of stake", + ) +} From e5d44b441029b356278d6d13549ff6c4fdb89291 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Fri, 23 Aug 2024 15:43:07 +0000 Subject: [PATCH 3/5] fix TODO: make method private addresses review comment https://github.com/ava-labs/awm-relayer/pull/457#discussion_r1728967766 --- signature-aggregator/aggregator/aggregator.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/signature-aggregator/aggregator/aggregator.go b/signature-aggregator/aggregator/aggregator.go index 161066fc..a48ac728 100644 --- a/signature-aggregator/aggregator/aggregator.go +++ b/signature-aggregator/aggregator/aggregator.go @@ -97,7 +97,7 @@ func (s *SignatureAggregator) CreateSignedMessage( var signingSubnet ids.ID var err error // If signingSubnet is not set we default to the subnet of the source blockchain - sourceSubnet, err := s.GetSubnetID(unsignedMessage.SourceChainID) + sourceSubnet, err := s.getSubnetID(unsignedMessage.SourceChainID) if err != nil { return nil, fmt.Errorf( "Source message subnet not found for chainID %s", @@ -343,9 +343,7 @@ func (s *SignatureAggregator) CreateSignedMessage( return nil, errNotEnoughSignatures } -// TODO: consider making this function private. its only reference seems to be -// within this module. -func (s *SignatureAggregator) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { +func (s *SignatureAggregator) getSubnetID(blockchainID ids.ID) (ids.ID, error) { s.subnetsMapLock.RLock() subnetID, ok := s.subnetIDsByBlockchainID[blockchainID] s.subnetsMapLock.RUnlock() @@ -357,13 +355,11 @@ func (s *SignatureAggregator) GetSubnetID(blockchainID ids.ID) (ids.ID, error) { if err != nil { return ids.ID{}, fmt.Errorf("source blockchain not found for chain ID %s", blockchainID) } - s.SetSubnetID(blockchainID, subnetID) + s.setSubnetID(blockchainID, subnetID) return subnetID, nil } -// TODO: consider making this function private. its only reference seems to be -// within this module. -func (s *SignatureAggregator) SetSubnetID(blockchainID ids.ID, subnetID ids.ID) { +func (s *SignatureAggregator) setSubnetID(blockchainID ids.ID, subnetID ids.ID) { s.subnetsMapLock.Lock() s.subnetIDsByBlockchainID[blockchainID] = subnetID s.subnetsMapLock.Unlock() From 44055292c5076c09171aba3976b489c5f0703bf4 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Thu, 29 Aug 2024 19:21:52 -0400 Subject: [PATCH 4/5] `name: Mock Checker` Co-authored-by: minghinmatthewlam Signed-off-by: F. Eugene Aumson --- .github/workflows/mock_checker.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mock_checker.yml b/.github/workflows/mock_checker.yml index 0454e1d4..ac422cf2 100644 --- a/.github/workflows/mock_checker.yml +++ b/.github/workflows/mock_checker.yml @@ -1,4 +1,4 @@ -name: Check generated code is up to date +name: Mock Checker on: push: From 9396b4b8f28370eaeb06eccd26cdde689051d8dc Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Fri, 30 Aug 2024 00:20:32 +0000 Subject: [PATCH 5/5] mv generate.sh invocation to test.sh addresses review comment https://github.com/ava-labs/awm-relayer/pull/457#discussion_r1736944784 --- scripts/build.sh | 3 --- scripts/test.sh | 6 ++++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/scripts/build.sh b/scripts/build.sh index a25a2ed6..3acd64a9 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -2,14 +2,11 @@ # Copyright (C) 2024, Ava Labs, Inc. All rights reserved. # See the file LICENSE for licensing terms. -set -e errexit - # Root directory root=$( cd "$(dirname "${BASH_SOURCE[0]}")" cd .. && pwd ) -"$root"/scripts/generate.sh "$root"/scripts/build_relayer.sh "$root"/scripts/build_signature_aggregator.sh diff --git a/scripts/test.sh b/scripts/test.sh index a25ba856..7204adca 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -27,12 +27,14 @@ if [ "$HELP" = true ]; then fi # Directory above this script -RELAYER_PATH=$( +root=$( cd "$(dirname "${BASH_SOURCE[0]}")" cd .. && pwd ) -source "$RELAYER_PATH"/scripts/constants.sh +source "$root"/scripts/constants.sh go build -o tests/cmd/decider/decider ./tests/cmd/decider/ +"$root"/scripts/generate.sh + go test -tags testing $VERBOSE ./...