From 8d11c10c12144a1e0d17bf5f70bb5bc98b3bf2f4 Mon Sep 17 00:00:00 2001 From: Reece Williams <31943163+Reecepbcups@users.noreply.github.com> Date: Tue, 21 Feb 2023 15:03:05 -0600 Subject: [PATCH] feat(x/feeshare): Allow registering factory contracts (#566) * Allow factory contracts to self register without bindings * cleanup factory logic * improve readability of isFactoryContract * Add register & update docs --- app/keepers/keepers.go | 4 +++ x/feeshare/README.md | 6 ++++ x/feeshare/client/cli/tx.go | 2 +- x/feeshare/keeper/msg_server.go | 53 ++++++++++++++++++++++++---- x/feeshare/keeper/msg_server_test.go | 21 +++++++++++ x/feeshare/spec/00_register.md | 31 ++++++++++++++++ x/feeshare/spec/00_update.md | 11 ++++++ x/feeshare/types/errors.go | 1 + 8 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 x/feeshare/spec/00_register.md create mode 100644 x/feeshare/spec/00_update.md diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 41e9deb32..431dfea5b 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -403,10 +403,14 @@ func NewAppKeepers( // Stargate Queries accepted := wasmkeeper.AcceptedStargateQueries{ + // ibc "/ibc.core.client.v1.Query/ClientState": &ibcclienttypes.QueryClientStateResponse{}, "/ibc.core.client.v1.Query/ConsensusState": &ibcclienttypes.QueryConsensusStateResponse{}, "/ibc.core.connection.v1.Query/Connection": &ibcconnectiontypes.QueryConnectionResponse{}, + // governance + "/cosmos.gov.v1beta1.Query/Vote": &govtypes.QueryVoteResponse{}, + // token factory "/osmosis.tokenfactory.v1beta1.Query/Params": &tokenfactorytypes.QueryParamsResponse{}, "/osmosis.tokenfactory.v1beta1.Query/DenomAuthorityMetadata": &tokenfactorytypes.QueryDenomAuthorityMetadataResponse{}, diff --git a/x/feeshare/README.md b/x/feeshare/README.md index e81b98f3e..d3fa96a13 100644 --- a/x/feeshare/README.md +++ b/x/feeshare/README.md @@ -6,3 +6,9 @@ This module is a heavily modified fork of [evmos/x/revenue](https://github.com/e A big thanks go to the original authors. [FeeShare Spec](spec/README.md) + +--- + +> [Register a Contract](spec/00_register.md) + +> [Update Conrtact Withdraw Address](spec/00_update.md) diff --git a/x/feeshare/client/cli/tx.go b/x/feeshare/client/cli/tx.go index da0ee5d35..013ecfc40 100644 --- a/x/feeshare/client/cli/tx.go +++ b/x/feeshare/client/cli/tx.go @@ -108,7 +108,7 @@ func NewCancelFeeShare() *cobra.Command { // address of a contract for fee distribution func NewUpdateFeeShare() *cobra.Command { cmd := &cobra.Command{ - Use: "update [contract_bech32] [", + Use: "update [contract_bech32] [new_withdraw_bech32]", Short: "Update withdrawer address for a contract registered for feeshare distribution.", Long: "Update withdrawer address for a contract registered for feeshare distribution. \nOnly the contract admin can update the withdrawer address.", Args: cobra.ExactArgs(2), diff --git a/x/feeshare/keeper/msg_server.go b/x/feeshare/keeper/msg_server.go index 3e29819f1..9c49da0b7 100644 --- a/x/feeshare/keeper/msg_server.go +++ b/x/feeshare/keeper/msg_server.go @@ -6,11 +6,36 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + wasmTypes "github.com/CosmWasm/wasmd/x/wasm/types" "github.com/CosmosContracts/juno/v13/x/feeshare/types" ) var _ types.MsgServer = &Keeper{} +func (k Keeper) GetIfContractWasCreatedFromFactory(ctx sdk.Context, contract sdk.AccAddress, info *wasmTypes.ContractInfo) bool { + // This will allow ANYONE to register FeeShare funds to its own contract if it was created from a factory contract + // Note: if there is no admin but a creator made it, then the creator can register it how they wish + + creator, err := sdk.AccAddressFromBech32(info.Creator) + if err != nil { + return false + } + + isFactoryContract := false + + if len(info.Admin) == 0 { + isFactoryContract = k.wasmKeeper.HasContractInfo(ctx, creator) + } else { + admin, err := sdk.AccAddressFromBech32(info.Admin) + if err != nil { + return false + } + isFactoryContract = k.wasmKeeper.HasContractInfo(ctx, admin) + } + + return isFactoryContract +} + // GetContractAdminOrCreatorAddress ensures the deployer is the contract's admin OR creator if no admin is set for all msg_server feeshare functions. func (k Keeper) GetContractAdminOrCreatorAddress(ctx sdk.Context, contract sdk.AccAddress, deployer string) (sdk.AccAddress, error) { var controllingAccount sdk.AccAddress @@ -73,18 +98,34 @@ func (k Keeper) RegisterFeeShare( return nil, sdkerrors.Wrapf(types.ErrFeeShareAlreadyRegistered, "contract is already registered %s", contract) } - // Check that the person who signed the message is the wasm contract admin, if so return the deployer address - deployer, err := k.GetContractAdminOrCreatorAddress(ctx, contract, msg.DeployerAddress) - if err != nil { - return nil, err - } - // Get the withdraw address of the contract withdrawer, err := sdk.AccAddressFromBech32(msg.WithdrawerAddress) if err != nil { return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid withdrawer address %s", msg.WithdrawerAddress) } + var deployer sdk.AccAddress + + if k.GetIfContractWasCreatedFromFactory(ctx, contract, k.wasmKeeper.GetContractInfo(ctx, contract)) { + // Anyone is allowed to register a contract to itself if it was created from a factory contract + if msg.WithdrawerAddress != msg.ContractAddress { + return nil, sdkerrors.Wrapf(types.ErrFeeShareInvalidWithdrawer, "withdrawer address must be the same as the contract address if it is from a factory contract withdraw:%s contract:%s", msg.WithdrawerAddress, msg.ContractAddress) + } + + // set the deployer address to the contract address so it can self register + msg.DeployerAddress = msg.ContractAddress + deployer, err = sdk.AccAddressFromBech32(msg.DeployerAddress) + if err != nil { + return nil, err + } + } else { + // Check that the person who signed the message is the wasm contract admin or creator (if no admin) + deployer, err = k.GetContractAdminOrCreatorAddress(ctx, contract, msg.DeployerAddress) + if err != nil { + return nil, err + } + } + // prevent storing the same address for deployer and withdrawer feeshare := types.NewFeeShare(contract, deployer, withdrawer) k.SetFeeShare(ctx, feeshare) diff --git a/x/feeshare/keeper/msg_server_test.go b/x/feeshare/keeper/msg_server_test.go index b43a1f52b..73590c18a 100644 --- a/x/feeshare/keeper/msg_server_test.go +++ b/x/feeshare/keeper/msg_server_test.go @@ -111,6 +111,7 @@ func (s *IntegrationTestSuite) TestRegisterFeeShare() { _ = s.FundAccount(s.ctx, sender, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1_000_000)))) contractAddress := s.InstantiateContract(sender.String(), "") + contractAddress2 := s.InstantiateContract(contractAddress, contractAddress) _, _, withdrawer := testdata.KeyTestPubAddr() @@ -160,6 +161,26 @@ func (s *IntegrationTestSuite) TestRegisterFeeShare() { resp: &types.MsgRegisterFeeShareResponse{}, shouldErr: false, }, + { + desc: "Invalid withdraw address for factory contract", + msg: &types.MsgRegisterFeeShare{ + ContractAddress: contractAddress2, + DeployerAddress: sender.String(), + WithdrawerAddress: sender.String(), + }, + resp: &types.MsgRegisterFeeShareResponse{}, + shouldErr: true, + }, + { + desc: "Success register factory contract to itself", + msg: &types.MsgRegisterFeeShare{ + ContractAddress: contractAddress2, + DeployerAddress: sender.String(), + WithdrawerAddress: contractAddress2, + }, + resp: &types.MsgRegisterFeeShareResponse{}, + shouldErr: false, + }, } { tc := tc s.Run(tc.desc, func() { diff --git a/x/feeshare/spec/00_register.md b/x/feeshare/spec/00_register.md new file mode 100644 index 000000000..36b8167a6 --- /dev/null +++ b/x/feeshare/spec/00_register.md @@ -0,0 +1,31 @@ +# Register a contract + +`junod tx feeshare register [contract_bech32] [withdraw_bech32] --from [key]` + +Registers the withdrawal address for the given contract. + +## Parameters + +`contract_bech32 (string, required)`: The bech32 address of the contract whose interaction fees will be shared. + +`withdraw_bech32 (string, required)`: The bech32 address where the interaction fees will be sent every block. + +## Description + +This command registers the withdrawal address for the given contract. Any time a user interacts with your contract, the funds will be sent to the withdrawal address. It can be any valid address, such as a DAO, normal account, another contract, or a multi-sig. + +## Permissions + +This command can only be run by the admin of the contract. If there is no admin, then it can only be run by the contract creator. + +## Exceptions + +```text +withdraw_bech32 can not be the community pool (distribution) address. This is a limitation of the way the SDK handles this module account +``` + +```text +For contracts created or administered by a contract factory, the withdrawal address can only be the same as the contract address. This can be registered by anyone, but it's unchangeable. This is helpful for SubDAOs or public goods to save fees in the treasury. + +If you create a contract like this, it's best to create an execution method for withdrawing fees to an account. To do this, you'll need to save the withdrawal address in the contract's state before uploading a non-migratable contract. +``` diff --git a/x/feeshare/spec/00_update.md b/x/feeshare/spec/00_update.md new file mode 100644 index 000000000..4e4133028 --- /dev/null +++ b/x/feeshare/spec/00_update.md @@ -0,0 +1,11 @@ +# Update a Contract's Withdrawal Address + +This can be changed at any time so long as you are still the admin or creator of a contract with the command: + +`junod tx feeshare update [contract] [new_withdraw_address]` + +## Update Exception + +```text +This can not be done if the contract was created from or is administered by another contract (a contract factory). There is not currently a way for a contract to change its own withdrawal address directly. +``` diff --git a/x/feeshare/types/errors.go b/x/feeshare/types/errors.go index 8c9f90cb6..1984d89d1 100644 --- a/x/feeshare/types/errors.go +++ b/x/feeshare/types/errors.go @@ -11,4 +11,5 @@ var ( ErrFeeShareNoContractDeployed = sdkerrrors.Register(ModuleName, 3, "no contract deployed") ErrFeeShareContractNotRegistered = sdkerrrors.Register(ModuleName, 4, "no feeshare registered for contract") ErrFeeSharePayment = sdkerrrors.Register(ModuleName, 5, "feeshare payment error") + ErrFeeShareInvalidWithdrawer = sdkerrrors.Register(ModuleName, 6, "invalid withdrawer address") )