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

Initial signature aggregator unit tests #457

Merged
merged 7 commits into from
Aug 30, 2024
Merged

Initial signature aggregator unit tests #457

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c1d6b8c
extract interface
feuGeneA Aug 22, 2024
e586c92
add AppReqNet mocks and a few initial tests
feuGeneA Aug 22, 2024
e5d44b4
fix TODO: make method private
feuGeneA Aug 23, 2024
9c32d45
Merge branch 'main' into sig-agg-unit-tests
feuGeneA Aug 29, 2024
4405529
`name: Mock Checker`
feuGeneA Aug 29, 2024
9396b4b
mv generate.sh invocation to test.sh
feuGeneA Aug 30, 2024
ad87ee7
Merge branch 'main' into sig-agg-unit-tests
feuGeneA Aug 30, 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
35 changes: 35 additions & 0 deletions .github/workflows/mock_checker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Check generated code is up to date
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved

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
44 changes: 33 additions & 11 deletions peers/app_request_network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link

Choose a reason for hiding this comment

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

why is this included in the source file? I would expect the source file to not need to have knowledge of the mock file at all. Instead, can the mock file keep track of command needed to regenerate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's here because the mock is generated from this source file, which is represented on this line by $GOFILE. btw i took this from your example 😅 at https://github.com/ava-labs/awm-relayer/pull/21/files#diff-b7c4d6477bfb555408745965840c387dcfc91a74bcbdc3b30d31d594e9abc0e5R4

my understanding is that the mock source file is 100% generated so we can't really put anything in there.


package peers

import (
Expand Down Expand Up @@ -28,7 +30,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
Expand All @@ -44,7 +66,7 @@ func NewNetwork(
registerer prometheus.Registerer,
trackedSubnets set.Set[ids.ID],
cfg Config,
) (*AppRequestNetwork, error) {
) (AppRequestNetwork, error) {
logger := logging.NewLogger(
"p2p-network",
logging.NewWrappedCore(
Expand Down Expand Up @@ -98,7 +120,7 @@ func NewNetwork(

validatorClient := validators.NewCanonicalValidatorClient(logger, cfg.GetPChainAPI())

arNetwork := &AppRequestNetwork{
arNetwork := &appRequestNetwork{
network: testNetwork,
handler: handler,
infoAPI: infoAPI,
Expand All @@ -116,7 +138,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()

Expand Down Expand Up @@ -200,7 +222,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)
Expand Down Expand Up @@ -240,7 +262,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,
Expand All @@ -249,24 +271,24 @@ 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)
}

//
// 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)
}
128 changes: 128 additions & 0 deletions peers/mocks/mock_app_request_network.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions relayer/application_relayer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions relayer/main/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions relayer/network_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions scripts/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this not being included in the build script to make it run faster if iterating on some manual testing. That being said in that case you would probably be running a specific build script as well so feel free to drop if you disagree

"$root"/scripts/build_relayer.sh
"$root"/scripts/build_signature_aggregator.sh
15 changes: 15 additions & 0 deletions scripts/generate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 ./...
Loading