Skip to content

Commit

Permalink
create a grpc test scaffold (#468)
Browse files Browse the repository at this point in the history
* create a grpc test scaffold

* use grpc scaffolding in ccip tests

* move scaffolding to common location, fixup cruft in some tests

* address comments; make ctx narrowly scoped

* fix missing Close

* most narrow ctx scope for tests
  • Loading branch information
krehermann authored Apr 16, 2024
1 parent 9f5c48c commit ec6d48a
Show file tree
Hide file tree
Showing 23 changed files with 550 additions and 630 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (e *CommitProviderClient) NewCommitStoreReader(ctx context.Context, addr cc
return nil, fmt.Errorf("failed to lookup off ramp reader service at %d: %w", resp.CommitStoreReaderServiceId, err)
}
// need to wrap grpc commitStore into the desired interface
return NewCommitStoreReaderGRPCClient(commitStoreConn, e.BrokerExt), nil
return NewCommitStoreReaderGRPCClient(e.BrokerExt, commitStoreConn), nil
}

// NewOffRampReader implements types.CCIPCommitProvider.
Expand All @@ -76,7 +76,7 @@ func (e *CommitProviderClient) NewOffRampReader(ctx context.Context, addr ccipty
return nil, fmt.Errorf("failed to lookup off ramp reader service at %d: %w", resp.OfframpReaderServiceId, err)
}
// need to wrap grpc offRamp into the desired interface
return NewOffRampReaderGRPCClient(offRampConn, e.BrokerExt), nil
return NewOffRampReaderGRPCClient(e.BrokerExt, offRampConn), nil
}

// NewOnRampReader implements types.CCIPCommitProvider.
Expand Down
14 changes: 11 additions & 3 deletions pkg/loop/internal/relayer/pluginprovider/ext/ccip/commit_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/smartcontractkit/chainlink-common/pkg/loop/internal/goplugin"
"github.com/smartcontractkit/chainlink-common/pkg/loop/internal/net"
ccippb "github.com/smartcontractkit/chainlink-common/pkg/loop/internal/pb/ccip"
"github.com/smartcontractkit/chainlink-common/pkg/services"
Expand All @@ -30,11 +31,12 @@ type CommitStoreGRPCClient struct {
// marking it so that it is considered when we implement the proxy.
// the reason it may not need to change is that the gas estimator server is
// a static resource of the offramp reader server. It is not created directly by the client.
b *net.BrokerExt
b *net.BrokerExt
conn grpc.ClientConnInterface
}

func NewCommitStoreReaderGRPCClient(cc grpc.ClientConnInterface, brokerExt *net.BrokerExt) *CommitStoreGRPCClient {
return &CommitStoreGRPCClient{client: ccippb.NewCommitStoreReaderClient(cc), b: brokerExt}
func NewCommitStoreReaderGRPCClient(brokerExt *net.BrokerExt, cc grpc.ClientConnInterface) *CommitStoreGRPCClient {
return &CommitStoreGRPCClient{client: ccippb.NewCommitStoreReaderClient(cc), b: brokerExt, conn: cc}
}

// CommitStoreGRPCServer implements [ccippb.CommitStoreReaderServer] by wrapping a
Expand Down Expand Up @@ -79,6 +81,12 @@ func NewCommitStoreReaderGRPCServer(impl ccip.CommitStoreReader, brokerExt *net.
// ensure the types are satisfied
var _ ccippb.CommitStoreReaderServer = (*CommitStoreGRPCServer)(nil)
var _ ccip.CommitStoreReader = (*CommitStoreGRPCClient)(nil)
var _ goplugin.GRPCClientConn = (*CommitStoreGRPCClient)(nil)

// ClientConn implements goplugin.GRPCClientConn.
func (c *CommitStoreGRPCClient) ClientConn() grpc.ClientConnInterface {
return c.conn
}

// ChangeConfig implements ccip.CommitStoreReader.
func (c *CommitStoreGRPCClient) ChangeConfig(ctx context.Context, onchainConfig []byte, offchainConfig []byte) (ccip.Address, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (e *ExecProviderClient) NewCommitStoreReader(ctx context.Context, addr ccip
return nil, fmt.Errorf("failed to lookup off ramp reader service at %d: %w", resp.CommitStoreReaderServiceId, err)
}
// need to wrap grpc commitStore into the desired interface
commitStore := NewCommitStoreReaderGRPCClient(commitStoreConn, e.BrokerExt)
commitStore := NewCommitStoreReaderGRPCClient(e.BrokerExt, commitStoreConn)

return commitStore, nil
}
Expand All @@ -78,7 +78,7 @@ func (e *ExecProviderClient) NewOffRampReader(ctx context.Context, addr cciptype
return nil, fmt.Errorf("failed to lookup off ramp reader service at %d: %w", resp.OfframpReaderServiceId, err)
}
// need to wrap grpc offRamp into the desired interface
offRamp := NewOffRampReaderGRPCClient(offRampConn, e.BrokerExt)
offRamp := NewOffRampReaderGRPCClient(e.BrokerExt, offRampConn)

return offRamp, nil
}
Expand Down Expand Up @@ -169,6 +169,11 @@ func (e *ExecProviderClient) SourceNativeToken(ctx context.Context) (cciptypes.A
return cciptypes.Address(resp.NativeTokenAddress), nil
}

// Close implements types.CCIPExecProvider.
func (e *ExecProviderClient) Close() error {
return shutdownGRPCServer(context.Background(), e.grpcClient)
}

// ExecProviderServer is a server that wraps the custom methods of the [types.CCIPExecProvider]
// this is necessary because those method create new resources that need to be served by the broker
// when we are running in legacy mode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
// is hosted by the relayer
type CommitGasEstimatorGRPCClient struct {
client ccippb.GasPriceEstimatorCommitClient
conn grpc.ClientConnInterface
}

func NewCommitGasEstimatorGRPCClient(cc grpc.ClientConnInterface) *CommitGasEstimatorGRPCClient {
Expand All @@ -43,6 +44,14 @@ func NewCommitGasEstimatorGRPCServer(impl cciptypes.GasPriceEstimatorCommit) *Co
var _ cciptypes.GasPriceEstimatorCommit = (*CommitGasEstimatorGRPCClient)(nil)
var _ ccippb.GasPriceEstimatorCommitServer = (*CommitGasEstimatorGRPCServer)(nil)

func (c *CommitGasEstimatorGRPCClient) ClientConn() grpc.ClientConnInterface {
return c.conn
}

func (c *CommitGasEstimatorGRPCClient) Close() error {
return nil
}

// DenoteInUSD implements ccip.GasPriceEstimatorCommit.
func (c *CommitGasEstimatorGRPCClient) DenoteInUSD(p *big.Int, wrappedNativePrice *big.Int) (*big.Int, error) {
resp, err := c.client.DenoteInUSD(context.Background(), &ccippb.DenoteInUSDRequest{
Expand Down Expand Up @@ -131,10 +140,11 @@ func (c *CommitGasEstimatorGRPCServer) Median(ctx context.Context, req *ccippb.M
// is hosted by the relayer
type ExecGasEstimatorGRPCClient struct {
client ccippb.GasPriceEstimatorExecClient
conn grpc.ClientConnInterface
}

func NewExecGasEstimatorGRPCClient(cc grpc.ClientConnInterface) *ExecGasEstimatorGRPCClient {
return &ExecGasEstimatorGRPCClient{client: ccippb.NewGasPriceEstimatorExecClient(cc)}
return &ExecGasEstimatorGRPCClient{client: ccippb.NewGasPriceEstimatorExecClient(cc), conn: cc}
}

// ExecGasEstimatorGRPCServer implements [ccippb.ExecGasEstimatorReaderServer] by wrapping a
Expand All @@ -155,6 +165,14 @@ func NewExecGasEstimatorGRPCServer(impl cciptypes.GasPriceEstimatorExec) *ExecGa
var _ cciptypes.GasPriceEstimatorExec = (*ExecGasEstimatorGRPCClient)(nil)
var _ ccippb.GasPriceEstimatorExecServer = (*ExecGasEstimatorGRPCServer)(nil)

func (e *ExecGasEstimatorGRPCClient) ClientConn() grpc.ClientConnInterface {
return nil
}

func (e *ExecGasEstimatorGRPCClient) Close() error {
return nil
}

// DenoteInUSD implements ccip.GasPriceEstimatorExec.
func (e *ExecGasEstimatorGRPCClient) DenoteInUSD(p *big.Int, wrappedNativePrice *big.Int) (*big.Int, error) {
resp, err := e.client.DenoteInUSD(context.Background(), &ccippb.DenoteInUSDRequest{
Expand Down
10 changes: 8 additions & 2 deletions pkg/loop/internal/relayer/pluginprovider/ext/ccip/offramp.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ type OffRampReaderGRPCClient struct {
// the reason it may not need to change is that the gas estimator server is
// a static resource of the offramp reader server. It is not created directly by the client.
b *net.BrokerExt

conn grpc.ClientConnInterface
}

// NewOffRampReaderGRPCClient creates a new OffRampReaderGRPCClient. It is used by the reporting plugin to call the offramp reader service.
// The client is created by wrapping a grpc client connection. It requires a brokerExt to allocate and serve the gas estimator server.
// *must* be the same broker used by the server BCF-3061
func NewOffRampReaderGRPCClient(cc grpc.ClientConnInterface, brokerExt *net.BrokerExt) *OffRampReaderGRPCClient {
return &OffRampReaderGRPCClient{client: ccippb.NewOffRampReaderClient(cc), b: brokerExt}
func NewOffRampReaderGRPCClient(brokerExt *net.BrokerExt, cc grpc.ClientConnInterface) *OffRampReaderGRPCClient {
return &OffRampReaderGRPCClient{client: ccippb.NewOffRampReaderClient(cc), b: brokerExt, conn: cc}
}

// OffRampReaderGRPCServer implements [ccippb.OffRampReaderServer] by wrapping a [cciptypes.OffRampReader] implementation.
Expand Down Expand Up @@ -109,6 +111,10 @@ func (o *OffRampReaderGRPCClient) ChangeConfig(ctx context.Context, onchainConfi
return cciptypes.Address(resp.OnchainConfigAddress), cciptypes.Address(resp.OffchainConfigAddress), nil
}

func (o *OffRampReaderGRPCClient) ClientConn() grpc.ClientConnInterface {
return o.conn
}

func (o *OffRampReaderGRPCClient) Close() error {
return shutdownGRPCServer(context.Background(), o.client)
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/loop/internal/relayer/pluginprovider/ext/ccip/onramp.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var _ cciptypes.OnRampReader = (*OnRampReaderGRPCClient)(nil)

type OnRampReaderGRPCClient struct {
grpc ccippb.OnRampReaderClient
conn grpc.ClientConnInterface
}

func NewOnRampReaderGRPCClient(cc grpc.ClientConnInterface) *OnRampReaderGRPCClient {
Expand All @@ -33,6 +34,10 @@ func (o *OnRampReaderGRPCClient) Address(ctx context.Context) (cciptypes.Address
return cciptypes.Address(resp.Address), nil
}

func (o *OnRampReaderGRPCClient) ClientConn() grpc.ClientConnInterface {
return o.conn
}

func (o *OnRampReaderGRPCClient) Close() error {
return shutdownGRPCServer(context.Background(), o.grpc)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"time"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/timestamppb"

Expand All @@ -24,6 +22,7 @@ import (
// is hosted by the relayer
type PriceRegistryGRPCClient struct {
grpc ccippb.PriceRegistryReaderClient
conn grpc.ClientConnInterface
}

func NewPriceRegistryGRPCClient(cc grpc.ClientConnInterface) *PriceRegistryGRPCClient {
Expand Down Expand Up @@ -60,15 +59,13 @@ func (p *PriceRegistryGRPCClient) Address(ctx context.Context) (cciptypes.Addres
return cciptypes.Address(resp.Address), nil
}

func (p *PriceRegistryGRPCClient) ClientConn() grpc.ClientConnInterface {
return p.conn
}

// Close implements ccip.PriceRegistryReader.
func (p *PriceRegistryGRPCClient) Close() error {
_, err := p.grpc.Close(context.Background(), &emptypb.Empty{})
// due to the onClose handler in the server, it may shutdown before it sends a response to client
// in that case, we expect the client to receive an Unavailable or Internal error
if status.Code(err) == codes.Unavailable || status.Code(err) == codes.Internal {
return nil
}
return err
return shutdownGRPCServer(context.Background(), p.grpc)
}

// GetFeeTokens implements ccip.PriceRegistryReader.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ import (
// is hosted by the relayer
type PriceGetterGRPCClient struct {
grpc ccippb.PriceGetterClient
conn grpc.ClientConnInterface
}

func NewPriceGetterGRPCClient(cc grpc.ClientConnInterface) *PriceGetterGRPCClient {
return &PriceGetterGRPCClient{grpc: ccippb.NewPriceGetterClient(cc)}
return &PriceGetterGRPCClient{grpc: ccippb.NewPriceGetterClient(cc), conn: cc}
}

// PriceGetterGRPCServer implements [ccippb.PriceGetterServer] by wrapping a
Expand All @@ -46,6 +47,10 @@ func NewPriceGetterGRPCServer(impl cciptypes.PriceGetter) *PriceGetterGRPCServer
var _ cciptypes.PriceGetter = (*PriceGetterGRPCClient)(nil)
var _ ccippb.PriceGetterServer = (*PriceGetterGRPCServer)(nil)

func (p *PriceGetterGRPCClient) ClientConn() grpc.ClientConnInterface {
return p.conn
}

// FilterConfiguredTokens implements ccip.PriceGetter.
func (p *PriceGetterGRPCClient) FilterConfiguredTokens(ctx context.Context, tokens []cciptypes.Address) (configured []cciptypes.Address, unconfigured []cciptypes.Address, err error) {
configured = []cciptypes.Address{}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
package test

import (
"context"
"fmt"
"net"
"sync"
"testing"

"github.com/hashicorp/consul/sdk/freeport"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"

loopnet "github.com/smartcontractkit/chainlink-common/pkg/loop/internal/net"
ccippb "github.com/smartcontractkit/chainlink-common/pkg/loop/internal/pb/ccip"
"github.com/smartcontractkit/chainlink-common/pkg/loop/internal/relayer/pluginprovider/ext/ccip"
looptest "github.com/smartcontractkit/chainlink-common/pkg/loop/internal/test"
"github.com/smartcontractkit/chainlink-common/pkg/utils/tests"
)

Expand All @@ -33,45 +29,19 @@ func TestStaticCommitGasEstimator(t *testing.T) {

func TestGasPriceEstimatorCommitGRPC(t *testing.T) {
t.Parallel()
ctx := tests.Context(t)
// create a price registry server
port := freeport.GetOne(t)
addr := fmt.Sprintf("localhost:%d", port)
lis, err := net.Listen("tcp", addr)
require.NoError(t, err, "failed to listen on port %d", port)
t.Cleanup(func() { lis.Close() })
// we explicitly stop the server later, do not add a cleanup function here
testServer := grpc.NewServer()
defer testServer.Stop()
// handle client close and server stop

gasPriceEstimatorCommit := ccip.NewCommitGasEstimatorGRPCServer(GasPriceEstimatorCommit)

ccippb.RegisterGasPriceEstimatorCommitServer(testServer, gasPriceEstimatorCommit)
// start the server and shutdown handler
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
require.NoError(t, testServer.Serve(lis))
}()

// create a price registry client
conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.NoError(t, err, "failed to dial %s", addr)
t.Cleanup(func() { conn.Close() })
client := ccip.NewCommitGasEstimatorGRPCClient(conn)

scaffold := looptest.NewGRPCScaffold(t, setupCommitGasEstimatorServer, setupCommitGasEstimatorClient)
t.Cleanup(scaffold.Close)
// test the client
roundTripGasPriceEstimatorCommitTests(ctx, t, client)
roundTripGasPriceEstimatorCommitTests(t, scaffold.Client())
}

// roundTripGasPriceEstimatorCommitTests tests the round trip of the client<->server.
// it should exercise all the methods of the client.
// do not add client.Close to this test, test that from the driver test
func roundTripGasPriceEstimatorCommitTests(ctx context.Context, t *testing.T, client *ccip.CommitGasEstimatorGRPCClient) {
func roundTripGasPriceEstimatorCommitTests(t *testing.T, client *ccip.CommitGasEstimatorGRPCClient) {
t.Run("GetGasPrice", func(t *testing.T) {
price, err := client.GetGasPrice(ctx)
price, err := client.GetGasPrice(tests.Context(t))
require.NoError(t, err)
assert.Equal(t, GasPriceEstimatorCommit.getGasPriceResponse, price)
})
Expand Down Expand Up @@ -100,3 +70,14 @@ func roundTripGasPriceEstimatorCommitTests(ctx context.Context, t *testing.T, cl
assert.Equal(t, GasPriceEstimatorCommit.medianResponse, median)
})
}

func setupCommitGasEstimatorServer(t *testing.T, s *grpc.Server, b *loopnet.BrokerExt) *ccip.CommitGasEstimatorGRPCServer {
gasProvider := ccip.NewCommitGasEstimatorGRPCServer(GasPriceEstimatorCommit)
ccippb.RegisterGasPriceEstimatorCommitServer(s, gasProvider)
return gasProvider
}

// adapt the client constructor so we can use it with the grpc scaffold
func setupCommitGasEstimatorClient(b *loopnet.BrokerExt, conn grpc.ClientConnInterface) *ccip.CommitGasEstimatorGRPCClient {
return ccip.NewCommitGasEstimatorGRPCClient(conn)
}
Loading

0 comments on commit ec6d48a

Please sign in to comment.