Skip to content

Commit

Permalink
feat(cosmic-swingset): Use vat cleanup budget values to allow slow cl…
Browse files Browse the repository at this point in the history
…eanup (#10165)

Fixes #8928

## Description
Adds new cosmos parameters for controlling terminated vat cleanup and uses them in cosmic-swingset to allow budget-limited cleanup in otherwise empty blocks. Also includes a fair amount of refactoring in support of the same, so best reviewed as individual commits.

### Security Considerations
Too-high budget values are a denial of service risk. All values are currently defaulted to zero for ~~no~~ minimal cleanup, but we may instead want to use small values.

### Scaling Considerations
See above.

### Documentation Considerations
Hmm, do we have documentation of cosmos-sdk parameters anywhere?

### Testing Considerations
Help wanted; the fully-threaded pathway is end-to-end but we may be able to do something more tractable. The policy logic is somewhat intricate and should be covered by tests.

### Upgrade Considerations
Upgrade will set the new parameters to their default values as defined in default-params.go. If we set non-zero defaults, then work could commence as soon as this code is live, which means it requires a chain-halting upgrade.
  • Loading branch information
mergify[bot] authored Nov 9, 2024
2 parents 02c06c4 + db8108a commit 2837beb
Show file tree
Hide file tree
Showing 40 changed files with 2,550 additions and 429 deletions.
4 changes: 2 additions & 2 deletions golang/cosmos/ante/inbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,14 @@ func (msk mockSwingsetKeeper) GetBeansPerUnit(ctx sdk.Context) map[string]sdkmat
return nil
}

func (msk mockSwingsetKeeper) ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdkmath.Uint) error {
func (msk mockSwingsetKeeper) ChargeBeans(ctx sdk.Context, beansPerUnit map[string]sdkmath.Uint, addr sdk.AccAddress, beans sdkmath.Uint) error {
return fmt.Errorf("not implemented")
}

func (msk mockSwingsetKeeper) GetSmartWalletState(ctx sdk.Context, addr sdk.AccAddress) swingtypes.SmartWalletState {
panic(fmt.Errorf("not implemented"))
}

func (msk mockSwingsetKeeper) ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error {
func (msk mockSwingsetKeeper) ChargeForSmartWallet(ctx sdk.Context, beansPerUnit map[string]sdkmath.Uint, addr sdk.AccAddress) error {
return fmt.Errorf("not implemented")
}
25 changes: 25 additions & 0 deletions golang/cosmos/proto/agoric/swingset/swingset.proto
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ message Params {
repeated QueueSize queue_max = 5 [
(gogoproto.nullable) = false
];

// Vat cleanup budget values.
// These values are used by SwingSet to control the pace of removing data
// associated with a terminated vat as described at
// https://github.com/Agoric/agoric-sdk/blob/master/packages/SwingSet/docs/run-policy.md#terminated-vat-cleanup
//
// There is no required order to this list of entries, but all the chain
// nodes must all serialize and deserialize the existing order without
// permuting it.
repeated UintMapEntry vat_cleanup_budget = 6 [
(gogoproto.nullable) = false
];
}

// The current state of the module.
Expand Down Expand Up @@ -119,6 +131,7 @@ message PowerFlagFee {
}

// Map element of a string key to a size.
// TODO: Replace with UintMapEntry?
message QueueSize {
option (gogoproto.equal) = true;

Expand All @@ -129,6 +142,18 @@ message QueueSize {
int32 size = 2;
}

// Map element of a string key to an unsigned integer.
// The value uses cosmos-sdk Uint rather than a native Go type to ensure that
// zeroes survive "omitempty" JSON serialization.
message UintMapEntry {
option (gogoproto.equal) = true;
string key = 1;
string value = 2 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Uint",
(gogoproto.nullable) = false
];
}

// Egress is the format for a swingset egress.
message Egress {
option (gogoproto.equal) = false;
Expand Down
23 changes: 16 additions & 7 deletions golang/cosmos/x/swingset/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,10 @@ func (k Keeper) BlockingSend(ctx sdk.Context, action vm.Action) (string, error)
}

func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
k.paramSpace.GetParamSet(ctx, &params)
// Note the use of "IfExists"...
// migration fills in missing data with defaults,
// so it is the only consumer that should ever see a nil pair.
k.paramSpace.GetParamSetIfExists(ctx, &params)
return params
}

Expand Down Expand Up @@ -304,9 +307,12 @@ func (k Keeper) SetBeansOwing(ctx sdk.Context, addr sdk.AccAddress, beans sdkmat
// ChargeBeans charges the given address the given number of beans. It divides
// the beans into the number to debit immediately vs. the number to store in the
// beansOwing.
func (k Keeper) ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdkmath.Uint) error {
beansPerUnit := k.GetBeansPerUnit(ctx)

func (k Keeper) ChargeBeans(
ctx sdk.Context,
beansPerUnit map[string]sdkmath.Uint,
addr sdk.AccAddress,
beans sdkmath.Uint,
) error {
wasOwing := k.GetBeansOwing(ctx, addr)
nowOwing := wasOwing.Add(beans)

Expand Down Expand Up @@ -339,10 +345,13 @@ func (k Keeper) ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdkmath.
}

// ChargeForSmartWallet charges the fee for provisioning a smart wallet.
func (k Keeper) ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error {
beansPerUnit := k.GetBeansPerUnit(ctx)
func (k Keeper) ChargeForSmartWallet(
ctx sdk.Context,
beansPerUnit map[string]sdkmath.Uint,
addr sdk.AccAddress,
) error {
beans := beansPerUnit[types.BeansPerSmartWalletProvision]
err := k.ChargeBeans(ctx, addr, beans)
err := k.ChargeBeans(ctx, beansPerUnit, addr, beans)
if err != nil {
return err
}
Expand Down
36 changes: 31 additions & 5 deletions golang/cosmos/x/swingset/types/default-params.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,23 @@ const (
BeansPerXsnapComputron = "xsnapComputron"
BeansPerSmartWalletProvision = "smartWalletProvision"

// PowerFlags.
PowerFlagSmartWallet = "SMART_WALLET"

// QueueSize keys.
// Keep up-to-date with updateQueueAllowed() in packanges/cosmic-swingset/src/launch-chain.js
// Keep up-to-date with updateQueueAllowed() in packages/cosmic-swingset/src/launch-chain.js
QueueInbound = "inbound"
QueueInboundMempool = "inbound_mempool"

// PowerFlags.
PowerFlagSmartWallet = "SMART_WALLET"
// Vat cleanup budget keys.
// Keep up-to-date with CleanupBudget in packages/cosmic-swingset/src/launch-chain.js
VatCleanupDefault = "default"
VatCleanupExports = "exports"
VatCleanupImports = "imports"
VatCleanupPromises = "promises"
VatCleanupKv = "kv"
VatCleanupSnapshots = "snapshots"
VatCleanupTranscripts = "transcripts"
)

var (
Expand Down Expand Up @@ -62,10 +72,26 @@ var (
}

DefaultInboundQueueMax = int32(1_000)

DefaultQueueMax = []QueueSize{
DefaultQueueMax = []QueueSize{
NewQueueSize(QueueInbound, DefaultInboundQueueMax),
}

DefaultVatCleanupDefault = sdk.NewUint(5)
// DefaultVatCleanupExports = DefaultVatCleanupDefault
// DefaultVatCleanupImports = DefaultVatCleanupDefault
// DefaultVatCleanupPromises = DefaultVatCleanupDefault
DefaultVatCleanupKv = sdk.NewUint(50)
// DefaultVatCleanupSnapshots = DefaultVatCleanupDefault
// DefaultVatCleanupTranscripts = DefaultVatCleanupDefault
DefaultVatCleanupBudget = []UintMapEntry{
UintMapEntry{VatCleanupDefault, DefaultVatCleanupDefault},
// UintMapEntry{VatCleanupExports, DefaultVatCleanupExports},
// UintMapEntry{VatCleanupImports, DefaultVatCleanupImports},
// UintMapEntry{VatCleanupPromises, DefaultVatCleanupPromises},
UintMapEntry{VatCleanupKv, DefaultVatCleanupKv},
// UintMapEntry{VatCleanupSnapshots, DefaultVatCleanupSnapshots},
// UintMapEntry{VatCleanupTranscripts, DefaultVatCleanupTranscripts},
}
)

// move DefaultBeansPerUnit to a function to allow for boot overriding of the Default params
Expand Down
4 changes: 2 additions & 2 deletions golang/cosmos/x/swingset/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ type AccountKeeper interface {

type SwingSetKeeper interface {
GetBeansPerUnit(ctx sdk.Context) map[string]sdkmath.Uint
ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdkmath.Uint) error
ChargeBeans(ctx sdk.Context, beansPerUnit map[string]sdkmath.Uint, addr sdk.AccAddress, beans sdkmath.Uint) error
IsHighPriorityAddress(ctx sdk.Context, addr sdk.AccAddress) (bool, error)
GetSmartWalletState(ctx sdk.Context, addr sdk.AccAddress) SmartWalletState
ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error
ChargeForSmartWallet(ctx sdk.Context, beansPerUnit map[string]sdkmath.Uint, addr sdk.AccAddress) error
}
42 changes: 30 additions & 12 deletions golang/cosmos/x/swingset/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ import (
"strings"

sdkioerrors "cosmossdk.io/errors"
"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
)

const RouterKey = ModuleName // this was defined in your key.go file
Expand Down Expand Up @@ -56,16 +59,22 @@ const (

// Charge an account address for the beans associated with given messages and storage.
// See list of bean charges in default-params.go
func chargeAdmission(ctx sdk.Context, keeper SwingSetKeeper, addr sdk.AccAddress, msgs []string, storageLen uint64) error {
beansPerUnit := keeper.GetBeansPerUnit(ctx)
func chargeAdmission(
ctx sdk.Context,
keeper SwingSetKeeper,
beansPerUnit map[string]sdkmath.Uint,
addr sdk.AccAddress,
msgs []string,
storageLen uint64,
) error {
beans := beansPerUnit[BeansPerInboundTx]
beans = beans.Add(beansPerUnit[BeansPerMessage].MulUint64((uint64(len(msgs)))))
for _, msg := range msgs {
beans = beans.Add(beansPerUnit[BeansPerMessageByte].MulUint64(uint64(len(msg))))
}
beans = beans.Add(beansPerUnit[BeansPerStorageByte].MulUint64(storageLen))

return keeper.ChargeBeans(ctx, addr, beans)
return keeper.ChargeBeans(ctx, beansPerUnit, addr, beans)
}

// checkSmartWalletProvisioned verifies if a smart wallet message (MsgWalletAction
Expand All @@ -74,7 +83,12 @@ func chargeAdmission(ctx sdk.Context, keeper SwingSetKeeper, addr sdk.AccAddress
// provisioning fee is charged successfully.
// All messages for non-provisioned smart wallets allowed here will result in
// an auto-provision action generated by the msg server.
func checkSmartWalletProvisioned(ctx sdk.Context, keeper SwingSetKeeper, addr sdk.AccAddress) error {
func checkSmartWalletProvisioned(
ctx sdk.Context,
keeper SwingSetKeeper,
beansPerUnit map[string]sdkmath.Uint,
addr sdk.AccAddress,
) error {
walletState := keeper.GetSmartWalletState(ctx, addr)

switch walletState {
Expand All @@ -91,7 +105,7 @@ func checkSmartWalletProvisioned(ctx sdk.Context, keeper SwingSetKeeper, addr sd
// This is a separate charge from the smart wallet action which triggered the check
// TODO: Currently this call does not mark the smart wallet provisioning as
// pending, resulting in multiple provisioning charges for the owner.
return keeper.ChargeForSmartWallet(ctx, addr)
return keeper.ChargeForSmartWallet(ctx, beansPerUnit, addr)
}
}

Expand All @@ -118,7 +132,8 @@ func (msg MsgDeliverInbound) CheckAdmissibility(ctx sdk.Context, data interface{
}
*/

return chargeAdmission(ctx, keeper, msg.Submitter, msg.Messages, 0)
beansPerUnit := keeper.GetBeansPerUnit(ctx)
return chargeAdmission(ctx, keeper, beansPerUnit, msg.Submitter, msg.Messages, 0)
}

// GetInboundMsgCount implements InboundMsgCarrier.
Expand Down Expand Up @@ -184,12 +199,13 @@ func (msg MsgWalletAction) CheckAdmissibility(ctx sdk.Context, data interface{})
return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "data must be a SwingSetKeeper, not a %T", data)
}

err := checkSmartWalletProvisioned(ctx, keeper, msg.Owner)
beansPerUnit := keeper.GetBeansPerUnit(ctx)
err := checkSmartWalletProvisioned(ctx, keeper, beansPerUnit, msg.Owner)
if err != nil {
return err
}

return chargeAdmission(ctx, keeper, msg.Owner, []string{msg.Action}, 0)
return chargeAdmission(ctx, keeper, beansPerUnit, msg.Owner, []string{msg.Action}, 0)
}

// GetInboundMsgCount implements InboundMsgCarrier.
Expand Down Expand Up @@ -256,12 +272,13 @@ func (msg MsgWalletSpendAction) CheckAdmissibility(ctx sdk.Context, data interfa
return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "data must be a SwingSetKeeper, not a %T", data)
}

err := checkSmartWalletProvisioned(ctx, keeper, msg.Owner)
beansPerUnit := keeper.GetBeansPerUnit(ctx)
err := checkSmartWalletProvisioned(ctx, keeper, beansPerUnit, msg.Owner)
if err != nil {
return err
}

return chargeAdmission(ctx, keeper, msg.Owner, []string{msg.SpendAction}, 0)
return chargeAdmission(ctx, keeper, beansPerUnit, msg.Owner, []string{msg.SpendAction}, 0)
}

// GetInboundMsgCount implements InboundMsgCarrier.
Expand Down Expand Up @@ -373,7 +390,8 @@ func (msg MsgInstallBundle) CheckAdmissibility(ctx sdk.Context, data interface{}
if !ok {
return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "data must be a SwingSetKeeper, not a %T", data)
}
return chargeAdmission(ctx, keeper, msg.Submitter, []string{msg.Bundle}, msg.ExpectedUncompressedSize())
beansPerUnit := keeper.GetBeansPerUnit(ctx)
return chargeAdmission(ctx, keeper, beansPerUnit, msg.Submitter, []string{msg.Bundle}, msg.ExpectedUncompressedSize())
}

// GetInboundMsgCount implements InboundMsgCarrier.
Expand Down
Loading

0 comments on commit 2837beb

Please sign in to comment.