Skip to content

Commit

Permalink
feat(vbank): new param allowed_monitoring_accounts (#10488)
Browse files Browse the repository at this point in the history
closes: #8564
  
## Description

To make the Golang `x/vbank` more flexible, introduce an `allowed_monitoring_accounts` parameter.  The default at agd genesis, or when an existing chain does a software-upgrade to this agd, will prevent balance monitoring except to the  `vbank/provision` module account (`"agoric1megzytg65cyrgzs6fvzxgrcqvwwl7ugpt62346"`, according to `packages/internal/src/config.js`).  This provisioning account's balances need to be monitored for normal operation of `packages/inter-protocol/src/provisionPoolKit.js`.

This restriction will prevent the cause of #8564 (that the vast majority of balance monitoring updates were to the `vbank/reserve` module account, which did nothing with them), while making it possible to reënable balance updates when they can be reworked and deemed harmless.

### Security Considerations

n/a

### Scaling Considerations

Improves scalability if the chain is dealing with Cosmos x/bank sends (which are also triggered by IBC transfers) among many different accounts.

### Documentation Considerations

n/a

### Testing Considerations

The `"*"` wildcard-all short-circuit is not tested yet.

### Upgrade Considerations

No JS changes, and preserving backward compatibility in the bridge messages, so this should be safe to upgrade.
  • Loading branch information
mergify[bot] authored Nov 19, 2024
2 parents 6368fcd + 0e367d3 commit 23e127e
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 57 deletions.
7 changes: 7 additions & 0 deletions golang/cosmos/proto/agoric/vbank/vbank.proto
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ message Params {
int64 reward_smoothing_blocks = 3 [
(gogoproto.moretags) = "yaml:\"reward_smoothing_blocks\""
];

// allowed_monitoring_accounts is an array of account addresses that can be
// monitored for sends and receives. An element of `"*"` will permit any
// address.
repeated string allowed_monitoring_accounts = 4 [
(gogoproto.moretags) = "yaml:\"allowed_monitoring_accounts\""
];
}

// The current state of the module.
Expand Down
7 changes: 6 additions & 1 deletion golang/cosmos/x/vbank/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ entirely at the ERTP level.

- `feeCollectorName`: the module which handles fee distribution to stakers.
- `reward_epoch_duration_blocks`: the duration (in blocks) over which fees should be given to the fee collector.
- `per_epoch_reward_fraction`: a decimal of how much of the `GiveawayPool` is paid as validator rewards per epoch
- `allowed_monitoring_accounts`: an array of account addresses that can be
monitored for sends and receives, defaulting to
`[authtypes.NewModuleAddress(types.ProvisionPoolName)]`. An element of `"*"`
will permit any address.

## State

Expand All @@ -22,7 +27,7 @@ The Vbank module maintains no significant state, but will access stored state th

Purse operations which change the balance result in a downcall to this module to update the underlying account. A downcall is also made to query the account balance.

Upon an `EndBlock()` call, the module will scan the block for all `MsgSend` and `MsgMultiSend` events (see `cosmos-sdk/x/bank/spec/04_events.md`) and perform a `VBANK_BALANCE_UPDATE` upcall for all denominations held in *only the mentioned module accounts*.
Upon an `EndBlock()` call, the module will scan the block for all `MsgSend` and `MsgMultiSend` events (see `cosmos-sdk/x/bank/spec/04_events.md`) and perform a `VBANK_BALANCE_UPDATE` upcall for all denominations held in *only in the allowed_monitoring_accounts*.

The following fields are common to the Vbank messages:
- `"address"`, `"recipient"`, `"sender"`: account address as a bech32-encoded string
Expand Down
13 changes: 4 additions & 9 deletions golang/cosmos/x/vbank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/Agoric/agoric-sdk/golang/cosmos/x/vbank/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

vm "github.com/Agoric/agoric-sdk/golang/cosmos/vm"
Expand Down Expand Up @@ -88,17 +87,13 @@ func (k Keeper) GetModuleAccountAddress(ctx sdk.Context, name string) sdk.AccAdd
return acct.GetAddress()
}

func (k Keeper) IsModuleAccount(ctx sdk.Context, addr sdk.AccAddress) bool {
acc := k.accountKeeper.GetAccount(ctx, addr)
if acc == nil {
return false
}
_, ok := acc.(authtypes.ModuleAccountI)
return ok
func (k Keeper) IsAllowedMonitoringAccount(ctx sdk.Context, addr sdk.AccAddress) bool {
params := k.GetParams(ctx)
return params.IsAllowedMonitoringAccount(addr.String())
}

func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
k.paramSpace.GetParamSet(ctx, &params)
k.paramSpace.GetParamSetIfExists(ctx, &params)
return params
}

Expand Down
30 changes: 30 additions & 0 deletions golang/cosmos/x/vbank/keeper/migrations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package keeper

import (
"github.com/Agoric/agoric-sdk/golang/cosmos/x/vbank/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// Migrator handles in-place store migrations.
type Migrator struct {
keeper Keeper
}

// NewMigrator creates a new Migrator based on the keeper.
func NewMigrator(keeper Keeper) Migrator {
return Migrator{keeper: keeper}
}

// Migrate1to2 migrates from version 1 to 2.
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
params := m.keeper.GetParams(ctx)
if params.AllowedMonitoringAccounts != nil {
return nil
}

defaultParams := types.DefaultParams()
params.AllowedMonitoringAccounts = defaultParams.AllowedMonitoringAccounts
m.keeper.SetParams(ctx, params)

return nil
}
10 changes: 8 additions & 2 deletions golang/cosmos/x/vbank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (AppModule) Name() string {
return ModuleName
}

func (AppModule) ConsensusVersion() uint64 { return 1 }
func (AppModule) ConsensusVersion() uint64 { return 2 }

// BeginBlock implements the AppModule interface
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
Expand Down Expand Up @@ -157,7 +157,7 @@ NextEvent:
addressToUpdate = make(map[string]sdk.Coins, len(addressToUpdate))
for addr, denoms := range unfilteredAddresses {
accAddr, err := sdk.AccAddressFromBech32(addr)
if err == nil && am.keeper.IsModuleAccount(ctx, accAddr) {
if err == nil && am.keeper.IsAllowedMonitoringAccount(ctx, accAddr) {
// Pass through the module account.
addressToUpdate[addr] = denoms
}
Expand Down Expand Up @@ -204,6 +204,12 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
tx := &types.UnimplementedMsgServer{}
types.RegisterMsgServer(cfg.MsgServer(), tx)
types.RegisterQueryServer(cfg.QueryServer(), am.keeper)

m := keeper.NewMigrator(am.keeper)
err := cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2)
if err != nil {
panic(err)
}
}

// InitGenesis performs genesis initialization for the ibc-transfer module. It returns
Expand Down
45 changes: 43 additions & 2 deletions golang/cosmos/x/vbank/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,33 @@ import (
yaml "gopkg.in/yaml.v2"

sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
)

const AllowAllMonitoringAccountsPattern = "*"

// Parameter keys
var (
ParamStoreKeyRewardEpochDurationBlocks = []byte("reward_epoch_duration_blocks")
ParamStoreKeyRewardSmoothingBlocks = []byte("reward_smoothing_blocks")
ParamStoreKeyPerEpochRewardFraction = []byte("per_epoch_reward_fraction")
ParamStoreKeyAllowedMonitoringAccounts = []byte("allowed_monitoring_accounts")
)

// ParamKeyTable returns the parameter key table.
func ParamKeyTable() paramtypes.KeyTable {
return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
}

// DefaultParams returns default distribution parameters
// DefaultParams returns default parameters
func DefaultParams() Params {
provisionAddress := authtypes.NewModuleAddress(ProvisionPoolName)
return Params{
RewardEpochDurationBlocks: 0,
RewardSmoothingBlocks: 1,
PerEpochRewardFraction: sdk.OneDec(),
AllowedMonitoringAccounts: []string{provisionAddress.String()},
}
}

Expand Down Expand Up @@ -67,12 +73,27 @@ func (p Params) RewardRate(pool sdk.Coins, blocks int64) sdk.Coins {
return sdk.NewCoins(coins...)
}

// IsAllowedMonitoringAccount checks to see if a given address is allowed to monitor its balance.
func (p Params) IsAllowedMonitoringAccount(addr string) bool {
for _, pat := range p.AllowedMonitoringAccounts {
switch pat {
case AllowAllMonitoringAccountsPattern, addr:
// Got an AllowAll pattern or an exact match.
return true
}
}

// No match found.
return false
}

// ParamSetPairs returns the parameter set pairs.
func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
return paramtypes.ParamSetPairs{
paramtypes.NewParamSetPair(ParamStoreKeyRewardEpochDurationBlocks, &p.RewardEpochDurationBlocks, validateRewardEpochDurationBlocks),
paramtypes.NewParamSetPair(ParamStoreKeyRewardSmoothingBlocks, &p.RewardSmoothingBlocks, validateRewardSmoothingBlocks),
paramtypes.NewParamSetPair(ParamStoreKeyPerEpochRewardFraction, &p.PerEpochRewardFraction, validatePerEpochRewardFraction),
paramtypes.NewParamSetPair(ParamStoreKeyAllowedMonitoringAccounts, &p.AllowedMonitoringAccounts, validateAllowedMonitoringAccounts),
}
}

Expand All @@ -84,7 +105,12 @@ func (p Params) ValidateBasic() error {
if err := validatePerEpochRewardFraction(p.PerEpochRewardFraction); err != nil {
return err
}

if err := validateRewardSmoothingBlocks(p.RewardSmoothingBlocks); err != nil {
return err
}
if err := validateAllowedMonitoringAccounts(p.AllowedMonitoringAccounts); err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -130,3 +156,18 @@ func validatePerEpochRewardFraction(i interface{}) error {

return nil
}

func validateAllowedMonitoringAccounts(i interface{}) error {
v, ok := i.([]string)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
}

for a, acc := range v {
if acc == "" {
return fmt.Errorf("allowed monitoring accounts element[%d] cannot be empty", a)
}
}

return nil
}
Loading

0 comments on commit 23e127e

Please sign in to comment.