Skip to content

Commit

Permalink
[Supplier] Refactor/non custodial staking tests (#718)
Browse files Browse the repository at this point in the history
## Summary

This PR is a follow-up to #716.
* It refactors the existing unit/integration tests to add
`Supplier.OwnerAddress`
* Adapts the `Supplier` (un)staking and tests (staking config,
(un)staking CLI and keeper methods) to comply with the `Supplier`
non-custodial staking.

## Issue

- #493 

## Type of change

Select one or more:

- [ ] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

**Documentation changes** (only if making doc changes)
- [ ] `make docusaurus_start`; only needed if you make doc changes

**Local Testing** (only if making code changes)
- [x] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- See [quickstart
guide](https://dev.poktroll.com/developer_guide/quickstart) for
instructions

**PR Testing** (only if making code changes)
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.
- **THIS IS VERY EXPENSIVE**, so only do it after all the reviews are
complete.
- Optionally run `make trigger_ci` if you want to re-trigger tests
without any code changes
- If tests fail, try re-running failed tests only using the GitHub UI as
shown
[here](https://github.com/pokt-network/poktroll/assets/1892194/607984e9-0615-4569-9452-4c730190c1d2)


## Sanity Checklist

- [ ] I have tested my changes using the available tooling
- [ ] I have commented my code
- [ ] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
	- Enhanced readability of output messages in the Makefile.
- Added `OwnerAddress` to the `Supplier` data model for better ownership
tracking.
- Introduced support for both owner and operator addresses in various
staking and unstaking processes.

- **Bug Fixes**
- Updated tests for `MsgUnstakeSupplier` to correctly validate signer
addresses and enhance robustness.

- **Documentation**
- Improved test coverage and clarity for supplier-related functionality,
reflecting the new data model changes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
red-0ne authored Aug 13, 2024
1 parent d296b17 commit 0ad3cbf
Show file tree
Hide file tree
Showing 19 changed files with 965 additions and 344 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ warn_message_local_stress_test: ## Print a warning message when kicking off a lo
@echo "| |"
@echo "| 1. Review the # of suppliers & gateways in 'load-testing/localnet_loadtest_manifest.yaml' |"
@echo "| 2. Update 'localnet_config.yaml' to reflect what you found in (1) |"
@echo "| DEVELOPER_TIP: If you're operating off defaults, you'll likely need to update to 3 |"
@echo "| DEVELOPER_TIP: If you're operating off defaults, you'll likely need to update to 3 |"
@echo "| |"
@echo "| TODO_DOCUMENT(@okdas): Move this into proper documentation w/ clearer explanations |"
@echo "| |"
Expand All @@ -838,7 +838,7 @@ warn_flaky_tests: ## Print a warning message that some unit tests may be flaky
@echo "| |"
@echo "| 1. Our unit / integration tests are far from perfect & some are flaky |"
@echo "| 2. If you ran 'make go_develop_and_test' and a failure occurred, try to run: |"
@echo "| 'make test_all' once or twice more |"
@echo "| 'make test_all' once or twice more |"
@echo "| 3. If the same error persists, isolate it with 'go test -v ./path/to/failing/module |"
@echo "| |"
@echo "+-----------------------------------------------------------------------------------------------+"
Expand Down
8 changes: 4 additions & 4 deletions api/poktroll/supplier/tx.pulsar.go

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

3 changes: 2 additions & 1 deletion config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ genesis:
denom: upokt
supplier:
supplierList:
- address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj
- owner_address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj
address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj
services:
- service:
id: anvil
Expand Down
50 changes: 38 additions & 12 deletions e2e/tests/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ func (s *suite) TheUserStakesAWithUpoktForServiceFromTheAccount(actorType string
require.NoError(s, err, "error creating config file in %q", path.Join(os.TempDir(), configPathPattern))

// Write the config content to the file
configContent := s.getConfigFileContent(amount, actorType, serviceId)
accAddress := accNameToAddrMap[accName]
configContent := s.getConfigFileContent(amount, accAddress, accAddress, actorType, serviceId)
_, err = configFile.Write([]byte(configContent))
require.NoError(s, err, "error writing config file %q", configFile.Name())

Expand All @@ -307,7 +308,13 @@ func (s *suite) TheUserStakesAWithUpoktForServiceFromTheAccount(actorType string
s.pocketd.result = res
}

func (s *suite) getConfigFileContent(amount int64, actorType, serviceId string) string {
func (s *suite) getConfigFileContent(
amount int64,
ownerAddress,
operatorAddress,
actorType,
serviceId string,
) string {
var configContent string
switch actorType {
case "application":
Expand All @@ -318,13 +325,15 @@ func (s *suite) getConfigFileContent(amount int64, actorType, serviceId string)
amount, serviceId)
case "supplier":
configContent = fmt.Sprintf(`
owner_address: %s
operator_address: %s
stake_amount: %dupokt
services:
- service_id: %s
endpoints:
- publicly_exposed_url: http://relayminer:8545
rpc_type: json_rpc`,
amount, serviceId)
ownerAddress, operatorAddress, amount, serviceId)
default:
s.Fatalf("ERROR: unknown actor type %s", actorType)
}
Expand All @@ -333,15 +342,32 @@ func (s *suite) getConfigFileContent(amount int64, actorType, serviceId string)
}

func (s *suite) TheUserUnstakesAFromTheAccount(actorType string, accName string) {
args := []string{
"tx",
actorType,
fmt.Sprintf("unstake-%s", actorType),
"--from",
accName,
keyRingFlag,
chainIdFlag,
"-y",
var args []string

switch actorType {
case "supplier":
args = []string{
"tx",
actorType,
fmt.Sprintf("unstake-%s", actorType),
accNameToAddrMap[accName], // supplier owner or operator address
"--from",
accName,
keyRingFlag,
chainIdFlag,
"-y",
}
default:
args = []string{
"tx",
actorType,
fmt.Sprintf("unstake-%s", actorType),
"--from",
accName,
keyRingFlag,
chainIdFlag,
"-y",
}
}

res, err := s.pocketd.RunCommandOnHost("", args...)
Expand Down
2 changes: 1 addition & 1 deletion proto/poktroll/supplier/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ message MsgUnstakeSupplier {
option (cosmos.msg.v1.signer) = "signer";

string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the message signer (i.e. owner or operator)
string address = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the operator (i.e. provider, non-custodial)
string address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the operator (i.e. provider, non-custodial)
}

message MsgUnstakeSupplierResponse {}
Expand Down
5 changes: 3 additions & 2 deletions testutil/integration/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,9 @@ func NewCompleteIntegrationApp(t *testing.T) *App {
// Prepare the on-chain supplier
supplierStake := types.NewCoin("upokt", math.NewInt(1000000))
defaultSupplier := sharedtypes.Supplier{
Address: supplierAddr.String(),
Stake: &supplierStake,
OwnerAddress: supplierAddr.String(),
Address: supplierAddr.String(),
Stake: &supplierStake,
Services: []*sharedtypes.SupplierServiceConfig{
{
Service: &defaultService,
Expand Down
16 changes: 13 additions & 3 deletions testutil/keeper/supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (
type SupplierModuleKeepers struct {
*keeper.Keeper
types.SharedKeeper
// Tracks the amount of funds returned to the supplier owner when the supplier is unbonded.
SupplierUnstakedFundsMap map[string]int64
}

func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) {
Expand All @@ -51,11 +53,18 @@ func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) {
cdc := codec.NewProtoCodec(registry)
authority := authtypes.NewModuleAddress(govtypes.ModuleName)

// Set a simple map to track the where the supplier stake is returned when
// the supplier is unbonded.
supplierUnstakedFundsMap := make(map[string]int64)

ctrl := gomock.NewController(t)
mockBankKeeper := mocks.NewMockBankKeeper(ctrl)
mockBankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), types.ModuleName, gomock.Any()).AnyTimes()
mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), types.ModuleName, gomock.Any(), gomock.Any()).AnyTimes()
mockBankKeeper.EXPECT().SpendableCoins(gomock.Any(), gomock.Any()).AnyTimes()
mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), types.ModuleName, gomock.Any(), gomock.Any()).AnyTimes().
Do(func(ctx context.Context, module string, addr sdk.AccAddress, coins sdk.Coins) {
supplierUnstakedFundsMap[addr.String()] += coins[0].Amount.Int64()
})

// Construct a real shared keeper.
sharedKeeper := sharedkeeper.NewKeeper(
Expand Down Expand Up @@ -95,8 +104,9 @@ func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) {
serviceKeeper.SetService(ctx, sharedtypes.Service{Id: "svcId2"})

supplierModuleKeepers := SupplierModuleKeepers{
Keeper: &supplierKeeper,
SharedKeeper: sharedKeeper,
Keeper: &supplierKeeper,
SharedKeeper: sharedKeeper,
SupplierUnstakedFundsMap: supplierUnstakedFundsMap,
}

return supplierModuleKeepers, ctx
Expand Down
5 changes: 3 additions & 2 deletions testutil/keeper/tokenomics.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ func TokenomicsKeeperWithActorAddrs(t testing.TB) (

// Prepare the test supplier.
supplier := sharedtypes.Supplier{
Address: sample.AccAddress(),
Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(100000)},
OwnerAddress: sample.AccAddress(),
Address: sample.AccAddress(),
Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(100000)},
}

ctrl := gomock.NewController(t)
Expand Down
10 changes: 6 additions & 4 deletions testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,9 @@ func DefaultSupplierModuleGenesisState(t *testing.T, n int) *suppliertypes.Genes
svcId := fmt.Sprintf("svc%d", i)
stake := sdk.NewCoin("upokt", math.NewInt(int64(i)))
supplier := sharedtypes.Supplier{
Address: sample.AccAddress(),
Stake: &stake,
OwnerAddress: sample.AccAddress(),
Address: sample.AccAddress(),
Stake: &stake,
Services: []*sharedtypes.SupplierServiceConfig{
{
Service: &sharedtypes.Service{Id: svcId},
Expand Down Expand Up @@ -169,8 +170,9 @@ func SupplierModuleGenesisStateWithAddresses(t *testing.T, addresses []string) *
state := suppliertypes.DefaultGenesis()
for _, addr := range addresses {
supplier := sharedtypes.Supplier{
Address: addr,
Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(10000)},
OwnerAddress: sample.AccAddress(),
Address: addr,
Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(10000)},
Services: []*sharedtypes.SupplierServiceConfig{
{
Service: &sharedtypes.Service{Id: "svc1"},
Expand Down
Loading

0 comments on commit 0ad3cbf

Please sign in to comment.