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

feat: addresses module #291

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a8bcd0e
Initial proto
cbrit Mar 13, 2024
1814cf1
Generate addressmap proto bindings
cbrit Mar 13, 2024
6bc712c
Change module name to addresses to avoid key prefix collision
cbrit Mar 13, 2024
8d36899
WIP - addresses module
cbrit Mar 14, 2024
4b374a1
WIP - address module builds
cbrit Mar 14, 2024
d415759
Unit tests and refactoring
cbrit Mar 15, 2024
c62622e
CLI queries and tests
cbrit Mar 15, 2024
3ed4f58
tx CLI commands and unit tests
cbrit Mar 15, 2024
1dbb93c
Add addresses gen state to setup_test
cbrit Mar 15, 2024
abe2f37
Rename default genesis state method
cbrit Mar 15, 2024
87e5901
Addresses integration test
cbrit Mar 15, 2024
19729af
Fixing linting errors
cbrit Mar 15, 2024
426819d
More linting fixes
cbrit Mar 15, 2024
f1f06a8
More linting
cbrit Mar 15, 2024
8a6e97a
A few tweaks
cbrit Mar 15, 2024
9cffc23
Test pagination in query unit test
cbrit Mar 18, 2024
7b51fab
Review items
cbrit Mar 18, 2024
cb2fc5a
Merge branch 'main' into collin/addresses
cbrit Mar 18, 2024
0b5ec42
Merge branch 'main' into collin/addresses
cbrit Aug 7, 2024
b6f11d5
Compiler errors, proto errors. WIP - need to bump go version to 1.22
cbrit Aug 7, 2024
5e35ce0
Fix addresses test
cbrit Aug 7, 2024
113970c
Fix addresses test
cbrit Aug 7, 2024
7e89e5e
Make linter happy
cbrit Aug 19, 2024
bf31ad9
Merge branch 'main' into collin/addresses
cbrit Sep 25, 2024
8e3d78f
Unhappy path integration tests for x/addresses
cbrit Sep 25, 2024
34fdfc0
Unhandled error
cbrit Sep 25, 2024
1673648
Improved keeper unit tests
cbrit Sep 25, 2024
8ac8e30
More unit tests
cbrit Sep 25, 2024
eb632dd
Make CI linter happy
cbrit Sep 26, 2024
3143444
more linter stuff
cbrit Sep 26, 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
1 change: 1 addition & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ jobs:
"CellarFees",
"Incentives",
"Pubsub",
"Addresses",
]
steps:
- name: Set up Go 1.22
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ e2e_incentives_test: e2e_clean_slate
e2e_pubsub_test: e2e_clean_slate
@E2E_SKIP_CLEANUP=true integration_tests/integration_tests.test -test.failfast -test.v -test.run IntegrationTestSuite -testify.m TestPubsub || make -s fail

e2e_addresses_test: e2e_clean_slate
@E2E_SKIP_CLEANUP=true integration_tests/integration_tests.test -test.failfast -test.v -test.run IntegrationTestSuite -testify.m TestAddresses || make -s fail

fail:
@echo 'test failed; dumping container logs into ./testlogs for review'
@docker logs ethereum > testlogs/ethereum.log 2>&1 || true
Expand Down
17 changes: 17 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ import (
gravitykeeper "github.com/peggyjv/gravity-bridge/module/v4/x/gravity/keeper"
gravitytypes "github.com/peggyjv/gravity-bridge/module/v4/x/gravity/types"
appParams "github.com/peggyjv/sommelier/v7/app/params"
"github.com/peggyjv/sommelier/v7/x/addresses"
addresseskeeper "github.com/peggyjv/sommelier/v7/x/addresses/keeper"
addressestypes "github.com/peggyjv/sommelier/v7/x/addresses/types"
"github.com/peggyjv/sommelier/v7/x/auction"
auctionclient "github.com/peggyjv/sommelier/v7/x/auction/client"
auctionkeeper "github.com/peggyjv/sommelier/v7/x/auction/keeper"
Expand Down Expand Up @@ -201,6 +204,7 @@ var (
incentives.AppModuleBasic{},
auction.AppModuleBasic{},
pubsub.AppModuleBasic{},
addresses.AppModuleBasic{},
)

// module account permissions
Expand All @@ -219,6 +223,7 @@ var (
axelarcorktypes.ModuleName: nil,
auctiontypes.ModuleName: nil,
pubsubtypes.ModuleName: nil,
addressestypes.ModuleName: nil,
}

// module accounts that are allowed to receive tokens
Expand Down Expand Up @@ -276,6 +281,7 @@ type SommelierApp struct {
IncentivesKeeper incentiveskeeper.Keeper
AuctionKeeper auctionkeeper.Keeper
PubsubKeeper pubsubkeeper.Keeper
AddressesKeeper addresseskeeper.Keeper

// make capability scoped keepers public for test purposes (IBC only)
ScopedAxelarCorkKeeper capabilitykeeper.ScopedKeeper
Expand Down Expand Up @@ -343,6 +349,7 @@ func NewSommelierApp(
auctiontypes.StoreKey,
cellarfeestypes.StoreKey,
pubsubtypes.StoreKey,
addressestypes.StoreKey,
)
tkeys := sdk.NewTransientStoreKeys(paramstypes.TStoreKey)
memKeys := sdk.NewMemoryStoreKeys(capabilitytypes.MemStoreKey)
Expand Down Expand Up @@ -524,6 +531,10 @@ func NewSommelierApp(
app.CorkKeeper.Hooks(),
))

app.AddressesKeeper = *addresseskeeper.NewKeeper(
appCodec, keys[addressestypes.StoreKey], app.GetSubspace(addressestypes.ModuleName),
)

// register the proposal types
govRouter := govtypesv1beta1.NewRouter()
govRouter.AddRoute(govtypes.RouterKey, govtypesv1beta1.ProposalHandler).
Expand Down Expand Up @@ -604,6 +615,7 @@ func NewSommelierApp(
cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper),
auction.NewAppModule(app.AuctionKeeper, app.BankKeeper, app.AccountKeeper, appCodec),
pubsub.NewAppModule(appCodec, app.PubsubKeeper, app.StakingKeeper, app.GravityKeeper),
addresses.NewAppModule(appCodec, app.AddressesKeeper),
)

// During begin block slashing happens after distr.BeginBlocker so that
Expand Down Expand Up @@ -638,6 +650,7 @@ func NewSommelierApp(
cellarfeestypes.ModuleName,
auctiontypes.ModuleName,
pubsubtypes.ModuleName,
addressestypes.ModuleName,
)

// NOTE gov must come before staking
Expand Down Expand Up @@ -668,6 +681,7 @@ func NewSommelierApp(
cellarfeestypes.ModuleName,
auctiontypes.ModuleName,
pubsubtypes.ModuleName,
addressestypes.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
Expand Down Expand Up @@ -706,6 +720,7 @@ func NewSommelierApp(
cellarfeestypes.ModuleName,
auctiontypes.ModuleName,
pubsubtypes.ModuleName,
addressestypes.ModuleName,
)

app.mm.RegisterInvariants(&app.CrisisKeeper)
Expand Down Expand Up @@ -741,6 +756,7 @@ func NewSommelierApp(
cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper),
auction.NewAppModule(app.AuctionKeeper, app.BankKeeper, app.AccountKeeper, appCodec),
pubsub.NewAppModule(appCodec, app.PubsubKeeper, app.StakingKeeper, app.GravityKeeper),
addresses.NewAppModule(appCodec, app.AddressesKeeper),
)

app.sm.RegisterStoreDecoders()
Expand Down Expand Up @@ -992,6 +1008,7 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
paramsKeeper.Subspace(incentivestypes.ModuleName)
paramsKeeper.Subspace(auctiontypes.ModuleName)
paramsKeeper.Subspace(pubsubtypes.ModuleName)
paramsKeeper.Subspace(addressestypes.ModuleName)

return paramsKeeper
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module github.com/peggyjv/sommelier/v7

go 1.22

toolchain go1.22.2
toolchain go1.22.1

require (
cosmossdk.io/api v0.3.1
Expand Down
145 changes: 145 additions & 0 deletions integration_tests/addresses_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
package integration_tests

import (
"context"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/peggyjv/sommelier/v7/x/addresses/types"
)

func (s *IntegrationTestSuite) TestAddresses() {
s.Run("Bring up chain, submit, query, and remove address mappings", func() {
s.T().Log("Starting x/addresses tests")
val := s.chain.validators[0]
kb, err := val.keyring()
s.Require().NoError(err)
val0ClientCtx, err := s.chain.clientContext("tcp://localhost:26657", &kb, "val", val.address())
s.Require().NoError(err)
addressesQueryClient := types.NewQueryClient(val0ClientCtx)

orch := s.chain.orchestrators[0]
orchClientCtx, err := s.chain.clientContext("tcp://localhost:26657", orch.keyring, "orch", orch.address())
s.Require().NoError(err)

evmAddress := common.HexToAddress("0x1234567890123456789012345678901234567890")
cosmosAddress := orch.address()

addAddressMappingMsg, err := types.NewMsgAddAddressMapping(evmAddress, cosmosAddress)
s.Require().NoError(err)

s.T().Logf("Submitting mapping from %s to %s", evmAddress.Hex(), cosmosAddress.String())
_, err = s.chain.sendMsgs(*orchClientCtx, addAddressMappingMsg)
s.Require().NoError(err)

s.T().Log("Testing queries return expected addresses")
s.Require().Eventually(func() bool {
queryRes, err := addressesQueryClient.QueryAddressMappings(context.Background(), &types.QueryAddressMappingsRequest{})
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use Context with Timeout for Client Queries

Currently, the code uses context.Background() for client queries, which may lead to indefinite blocking if the service is unresponsive. Consider using a context with a timeout to prevent potential hangs:

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

// Example usage:
queryRes, err := addressesQueryClient.QueryAddressMappings(ctx, &types.QueryAddressMappingsRequest{})

Applying a timeout ensures that the test fails gracefully if a query does not complete within the expected time frame.

Also applies to: 46-46, 51-51, 65-65, 74-74, 136-136, 141-141

if err != nil {
s.T().Logf("Error querying address mappings: %s", err)
return false
}

return len(queryRes.AddressMappings) == 1 && evmAddress.Hex() == queryRes.AddressMappings[0].EvmAddress
}, 20*time.Second, 4*time.Second, "address mapping never found")

queryByEvmRes, err := addressesQueryClient.QueryAddressMappingByEVMAddress(context.Background(), &types.QueryAddressMappingByEVMAddressRequest{EvmAddress: evmAddress.Hex()})
s.Require().NoError(err)
s.Require().Equal(cosmosAddress.String(), queryByEvmRes.CosmosAddress, "Cosmos address does not match")
s.Require().Equal(evmAddress.Hex(), queryByEvmRes.EvmAddress, "EVM address does not match")

queryByCosmosRes, err := addressesQueryClient.QueryAddressMappingByCosmosAddress(context.Background(), &types.QueryAddressMappingByCosmosAddressRequest{CosmosAddress: cosmosAddress.String()})
s.Require().NoError(err)
s.Require().Equal(cosmosAddress.String(), queryByCosmosRes.CosmosAddress, "Cosmos address does not match")
s.Require().Equal(evmAddress.Hex(), queryByCosmosRes.EvmAddress, "EVM address does not match")

s.T().Log("Removing address mapping")
removeAddressMappingMsg, err := types.NewMsgRemoveAddressMapping(cosmosAddress)
s.Require().NoError(err)

_, err = s.chain.sendMsgs(*orchClientCtx, removeAddressMappingMsg)
s.Require().NoError(err)

s.T().Log("Testing mappings query returns no addresses")
s.Require().Eventually(func() bool {
queryRes, err := addressesQueryClient.QueryAddressMappings(context.Background(), &types.QueryAddressMappingsRequest{})
if err != nil {
s.T().Logf("Error querying address mappings: %s", err)
return false
}

return len(queryRes.AddressMappings) == 0
}, 20*time.Second, 4*time.Second, "address mapping not deleted")

_, err = addressesQueryClient.QueryAddressMappingByEVMAddress(context.Background(), &types.QueryAddressMappingByEVMAddressRequest{EvmAddress: evmAddress.Hex()})
s.Require().Error(err)
s.Require().Contains(err.Error(), "code = NotFound")

_, err = addressesQueryClient.QueryAddressMappingByCosmosAddress(context.Background(), &types.QueryAddressMappingByCosmosAddressRequest{CosmosAddress: cosmosAddress.String()})
s.Require().Error(err)
s.Require().Contains(err.Error(), "code = NotFound")

// Test error cases

// Test adding multiple mappings
s.T().Log("Adding multiple mappings")
evmAddress2 := common.HexToAddress("0x2345678901234567890123456789012345678901")
cosmosAddress2 := s.chain.orchestrators[1].address()
orch1 := s.chain.orchestrators[1]
orch1ClientCtx, err := s.chain.clientContext("tcp://localhost:26657", orch1.keyring, "orch", orch1.address())
s.Require().NoError(err)

addAddressMappingMsg2, err := types.NewMsgAddAddressMapping(evmAddress2, cosmosAddress2)
s.Require().NoError(err)

_, err = s.chain.sendMsgs(*orchClientCtx, addAddressMappingMsg)
s.Require().NoError(err)
_, err = s.chain.sendMsgs(*orch1ClientCtx, addAddressMappingMsg2)
s.Require().NoError(err)

// Query multiple mappings
s.T().Log("Querying multiple mappings")
s.Require().Eventually(func() bool {
queryRes, err := addressesQueryClient.QueryAddressMappings(context.Background(), &types.QueryAddressMappingsRequest{})
if err != nil {
s.T().Logf("Error querying address mappings: %s", err)
return false
}

return len(queryRes.AddressMappings) == 2
}, 20*time.Second, 4*time.Second, "expected two address mappings")

// Test adding a duplicate mapping
s.T().Log("Adding a duplicate mapping")
duplicateMsg, err := types.NewMsgAddAddressMapping(evmAddress, cosmosAddress)
s.Require().NoError(err)

_, err = s.chain.sendMsgs(*orchClientCtx, duplicateMsg)
s.Require().NoError(err)
_, err = s.chain.sendMsgs(*orchClientCtx, duplicateMsg)
s.Require().NoError(err)

// Test removing a non-existent mapping
s.T().Log("Removing a non-existent mapping")
nonExistentAddress := s.chain.orchestrators[2].address()
orch2 := s.chain.orchestrators[2]
orch2ClientCtx, err := s.chain.clientContext("tcp://localhost:26657", orch2.keyring, "orch", orch2.address())
s.Require().NoError(err)
removeNonExistentMsg, err := types.NewMsgRemoveAddressMapping(nonExistentAddress)
s.Require().NoError(err)

_, err = s.chain.sendMsgs(*orch2ClientCtx, removeNonExistentMsg)
s.Require().NoError(err)

// Test querying with invalid addresses
s.T().Log("Querying with invalid addresses")
_, err = addressesQueryClient.QueryAddressMappingByEVMAddress(context.Background(), &types.QueryAddressMappingByEVMAddressRequest{EvmAddress: "invalid"})
s.Require().Error(err)
s.Require().Contains(err.Error(), "invalid EVM address")

s.T().Log("Querying with invalid cosmos address")
_, err = addressesQueryClient.QueryAddressMappingByCosmosAddress(context.Background(), &types.QueryAddressMappingByCosmosAddressRequest{CosmosAddress: "invalid"})
s.Require().Error(err)
s.Require().Contains(err.Error(), "failed to parse cosmos address")
})
Comment on lines +12 to +144
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor Test into Sub-Tests for Improved Readability

The TestAddresses function contains multiple test scenarios, including adding, querying, and removing address mappings, as well as testing error cases. To enhance readability and maintainability, consider splitting these scenarios into separate sub-tests using s.Run():

func (s *IntegrationTestSuite) TestAddresses() {
    s.Run("Add and Query Address Mappings", func() {
        // Test logic for adding and querying mappings
    })

    s.Run("Remove Address Mappings", func() {
        // Test logic for removing mappings
    })

    s.Run("Error Cases", func() {
        // Test logic for error scenarios
    })
}

This approach isolates test cases, making it easier to identify and debug failures related to specific functionality.

}
6 changes: 6 additions & 0 deletions integration_tests/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
gravitytypes "github.com/peggyjv/gravity-bridge/module/v4/x/gravity/types"
"github.com/peggyjv/sommelier/v7/app/params"
addressestypes "github.com/peggyjv/sommelier/v7/x/addresses/types"
auctiontypes "github.com/peggyjv/sommelier/v7/x/auction/types"
axelarcorktypes "github.com/peggyjv/sommelier/v7/x/axelarcork/types"
cellarfeestypes "github.com/peggyjv/sommelier/v7/x/cellarfees/types"
Expand Down Expand Up @@ -482,6 +483,11 @@ func (s *IntegrationTestSuite) initGenesis() {
s.Require().NoError(err)
appGenState[pubsubtypes.ModuleName] = bz

// set addresses gen state
addressesGenState := addressestypes.DefaultGenesisState()
s.Require().NoError(cdc.UnmarshalJSON(appGenState[addressestypes.ModuleName], &addressesGenState))
appGenState[addressestypes.ModuleName] = cdc.MustMarshalJSON(&addressesGenState)

// generate genesis txs
genTxs := make([]json.RawMessage, len(s.chain.validators))
for i, val := range s.chain.validators {
Expand Down
10 changes: 10 additions & 0 deletions proto/addresses/v1/addresses.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
syntax = "proto3";
package addresses.v1;

option go_package = "github.com/peggyjv/sommelier/v7/x/addresses/types";

message AddressMapping {
string cosmos_address = 1;
string evm_address = 2;
}

15 changes: 15 additions & 0 deletions proto/addresses/v1/genesis.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
syntax = "proto3";
package addresses.v1;

option go_package = "github.com/peggyjv/sommelier/v7/x/addresses/types";

import "addresses/v1/addresses.proto";
import "gogoproto/gogo.proto";
import "google/api/annotations.proto";

message GenesisState {
Params params = 1 [ (gogoproto.nullable) = false ];
repeated AddressMapping address_mappings = 2;
}

message Params {}
63 changes: 63 additions & 0 deletions proto/addresses/v1/query.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
syntax = "proto3";

package addresses.v1;

import "addresses/v1/addresses.proto";
import "addresses/v1/genesis.proto";
import "gogoproto/gogo.proto";
import "google/api/annotations.proto";
import "cosmos/base/query/v1beta1/pagination.proto";


option go_package = "github.com/peggyjv/sommelier/v7/x/addresses/types";

service Query {
rpc QueryParams(QueryParamsRequest) returns (QueryParamsResponse) {
option (google.api.http).get = "/sommelier/addresses/v1/params";
}

rpc QueryAddressMappings(QueryAddressMappingsRequest) returns (QueryAddressMappingsResponse) {
option (google.api.http).get = "/sommelier/addresses/v1/address_mappings";
}

rpc QueryAddressMappingByEVMAddress(QueryAddressMappingByEVMAddressRequest) returns (QueryAddressMappingByEVMAddressResponse) {
option (google.api.http).get = "/sommelier/addresses/v1/address_mappings/evm/{evm_address}";
}

rpc QueryAddressMappingByCosmosAddress(QueryAddressMappingByCosmosAddressRequest) returns (QueryAddressMappingByCosmosAddressResponse) {
option (google.api.http).get = "/sommelier/addresses/v1/address_mappings/cosmos/{cosmos_address}";
}
}

message QueryParamsRequest {}

message QueryParamsResponse {
Params params = 1;
}

message QueryAddressMappingsRequest {
cosmos.base.query.v1beta1.PageRequest pagination = 1 [ (gogoproto.nullable) = false ];
}

message QueryAddressMappingsResponse {
repeated AddressMapping address_mappings = 1;
cosmos.base.query.v1beta1.PageResponse pagination = 2 [ (gogoproto.nullable) = false ];
}

message QueryAddressMappingByEVMAddressRequest {
string evm_address = 1;
}

message QueryAddressMappingByEVMAddressResponse {
string cosmos_address = 1;
string evm_address = 2;
}

message QueryAddressMappingByCosmosAddressRequest {
string cosmos_address = 1;
}

message QueryAddressMappingByCosmosAddressResponse {
string cosmos_address = 1;
string evm_address = 2;
}
Loading
Loading