From cf6f4d250afb29f958256d4e18f692640d1bdf75 Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Fri, 1 Dec 2023 18:48:51 +0000 Subject: [PATCH] fixup! feat(x/swingset): auto-provision smart wallet --- golang/cosmos/ante/inbound_test.go | 4 ++ golang/cosmos/x/swingset/keeper/keeper.go | 19 +++++++++ golang/cosmos/x/swingset/keeper/msg_server.go | 7 ++-- .../x/swingset/types/expected_keepers.go | 1 + golang/cosmos/x/swingset/types/msgs.go | 39 +++++++------------ 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/golang/cosmos/ante/inbound_test.go b/golang/cosmos/ante/inbound_test.go index 1fd52a0ffb28..2f3e42e38a90 100644 --- a/golang/cosmos/ante/inbound_test.go +++ b/golang/cosmos/ante/inbound_test.go @@ -221,3 +221,7 @@ func (msk mockSwingsetKeeper) ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, func (msk mockSwingsetKeeper) GetSmartWalletState(ctx sdk.Context, addr sdk.AccAddress) (swingtypes.SmartWalletState, error) { return swingtypes.SmartWalletStateUnspecified, fmt.Errorf("not implemented") } + +func (msk mockSwingsetKeeper) ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error { + return fmt.Errorf("not implemented") +} diff --git a/golang/cosmos/x/swingset/keeper/keeper.go b/golang/cosmos/x/swingset/keeper/keeper.go index a4352d191f82..038f8f5a6077 100644 --- a/golang/cosmos/x/swingset/keeper/keeper.go +++ b/golang/cosmos/x/swingset/keeper/keeper.go @@ -335,6 +335,25 @@ func (k Keeper) ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdk.Uint return nil } +// ChargeForSmartWallet charges the fee for provisioning a smart wallet. +func (k Keeper) ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error { + beansPerUnit := k.GetBeansPerUnit(ctx) + beans := beansPerUnit[types.BeansPerSmartWalletProvision] + err := k.ChargeBeans(ctx, addr, beans) + if err != nil { + return err + } + + // TODO: mark that a smart wallet provision is pending. However in that case, + // auto-provisioning should still be performed (but without fees being charged), + // until the controller actually provisions the smart wallet (the operation may + // transiently fail, requiring retries until success). + // However the provisioning code is not currently idempotent, and has side + // effects when the smart wallet is already provisioned. + + return nil +} + // makeFeeMenu returns a map from power flag to its fee. In the case of duplicates, the // first one wins. func makeFeeMenu(powerFlagFees []types.PowerFlagFee) map[string]sdk.Coins { diff --git a/golang/cosmos/x/swingset/keeper/msg_server.go b/golang/cosmos/x/swingset/keeper/msg_server.go index 4fec7e72cd7c..d89700fa1fc6 100644 --- a/golang/cosmos/x/swingset/keeper/msg_server.go +++ b/golang/cosmos/x/swingset/keeper/msg_server.go @@ -146,14 +146,13 @@ type provisionAction struct { // non-provisioned smart wallets allowed by the admission AnteHandler should // auto-provision the smart wallet. func (keeper msgServer) provisionIfNeeded(ctx sdk.Context, owner sdk.AccAddress) error { - // TODO: If/when we implement marking of pending smart wallet provisions, - // here we'll need to generate a provision action until the smart wallet has + // We need to generate a provision action until the smart wallet has // been fully provisioned by the controller. This is because a provision is // not guaranteed to succeed (e.g. lack of provision pool funds) - provisioned, err := keeper.HasSmartWallet(ctx, owner) + walletState, err := keeper.GetSmartWalletState(ctx, owner) if err != nil { return err - } else if provisioned { + } else if walletState == types.SmartWalletStateProvisioned { return nil } diff --git a/golang/cosmos/x/swingset/types/expected_keepers.go b/golang/cosmos/x/swingset/types/expected_keepers.go index 14abb2354311..1331e97cf9e0 100644 --- a/golang/cosmos/x/swingset/types/expected_keepers.go +++ b/golang/cosmos/x/swingset/types/expected_keepers.go @@ -25,4 +25,5 @@ type SwingSetKeeper interface { ChargeBeans(ctx sdk.Context, addr sdk.AccAddress, beans sdk.Uint) error IsHighPriorityAddress(ctx sdk.Context, addr sdk.AccAddress) (bool, error) GetSmartWalletState(ctx sdk.Context, addr sdk.AccAddress) (SmartWalletState, error) + ChargeForSmartWallet(ctx sdk.Context, addr sdk.AccAddress) error } diff --git a/golang/cosmos/x/swingset/types/msgs.go b/golang/cosmos/x/swingset/types/msgs.go index fef9c010ee4d..90f673284267 100644 --- a/golang/cosmos/x/swingset/types/msgs.go +++ b/golang/cosmos/x/swingset/types/msgs.go @@ -49,42 +49,33 @@ func chargeAdmission(ctx sdk.Context, keeper SwingSetKeeper, addr sdk.AccAddress } // checkSmartWalletProvisioned verifies if a smart wallet message can be -// delivered for the owner's address. A messaged is allowed if a smart wallet +// delivered for the owner's address. A message is allowed if a smart wallet // is already provisioned for the address, or if the 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 { - // This checks if a smart wallet has already been provisioned by the controller - // However a provision (either explicit or automatic) may be pending execution - // and this check will not catch that case, resulting in the owner being charged - // for provisioning again. - // Furthermore, while multiple swingset messages are not currently allowed in - // the same transaction, if that is ever relaxed, we would end up generating - // multiple provision actions, and the provisioning code is not fully - // idempotent. However this is only a degenerate case of the multiple txs case, - // and as such is charged again similarly. - // TODO: mark that a smart wallet provision is pending. However in that case, - // auto-provisioning should still be performed (but without fees being charged), - // until the controller actually provisions the smart wallet (the operation may - // transiently fail, requiring retries until success) walletState, err := keeper.GetSmartWalletState(ctx, addr) if err != nil { return err } - if !isProvisioned { - beansPerUnit := keeper.GetBeansPerUnit(ctx) - beans := beansPerUnit[BeansPerSmartWalletProvision] - + switch walletState { + case SmartWalletStateProvisioned: + // The address already has a smart wallet + return nil + case SmartWalletStatePending: + // A provision (either explicit or automatic) may be pending execution in + // the controller, or if we ever allow multiple swingset messages per + // transaction, a previous message may have provisioned the wallet. + return nil + default: + // Charge for the smart wallet. // This is a separate charge from the smart wallet action which triggered the check - err = keeper.ChargeBeans(ctx, addr, beans) - if err != nil { - return err - } + // 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 nil } func NewMsgDeliverInbound(msgs *Messages, submitter sdk.AccAddress) *MsgDeliverInbound {