diff --git a/interchaintest/module_feeshare_test.go b/interchaintest/module_feeshare_test.go index 098346a28..04a1bd663 100644 --- a/interchaintest/module_feeshare_test.go +++ b/interchaintest/module_feeshare_test.go @@ -19,7 +19,6 @@ func TestJunoFeeShare(t *testing.T) { // Chains juno := chains[0].(*cosmos.CosmosChain) - t.Log("juno.GetHostRPCAddress()", juno.GetHostRPCAddress()) nativeDenom := juno.Config().Denom diff --git a/x/feeshare/ante/ante.go b/x/feeshare/ante/ante.go index ef12d000c..52035e701 100644 --- a/x/feeshare/ante/ante.go +++ b/x/feeshare/ante/ante.go @@ -1,6 +1,8 @@ package ante import ( + "encoding/json" + wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" errorsmod "cosmossdk.io/errors" @@ -55,6 +57,11 @@ func FeePayLogic(fees sdk.Coins, govPercent sdk.Dec, numPairs int) sdk.Coins { return splitFees } +type FeeSharePayoutEventOutput struct { + WithdrawAddress sdk.AccAddress `json:"withdraw_address"` + FeesPaid sdk.Coins `json:"fees_paid"` +} + // FeeSharePayout takes the total fees and redistributes 50% (or param set) to the contract developers // provided they opted-in to payments. func FeeSharePayout(ctx sdk.Context, bankKeeper BankKeeper, totalFees sdk.Coins, revKeeper FeeShareKeeper, msgs []sdk.Msg) error { @@ -101,20 +108,37 @@ func FeeSharePayout(ctx sdk.Context, bankKeeper BankKeeper, totalFees sdk.Coins, } } - // FeeShare logic payouts for contracts numPairs := len(toPay) + + feesPaidOutput := make([]FeeSharePayoutEventOutput, numPairs) if numPairs > 0 { govPercent := params.DeveloperShares splitFees := FeePayLogic(fees, govPercent, numPairs) // pay fees evenly between all withdraw addresses - for _, withdrawAddr := range toPay { + for i, withdrawAddr := range toPay { err := bankKeeper.SendCoinsFromModuleToAccount(ctx, authtypes.FeeCollectorName, withdrawAddr, splitFees) + feesPaidOutput[i] = FeeSharePayoutEventOutput{ + WithdrawAddress: withdrawAddr, + FeesPaid: splitFees, + } + if err != nil { return errorsmod.Wrapf(feeshare.ErrFeeSharePayment, "failed to pay fees to contract developer: %s", err.Error()) } } } + bz, err := json.Marshal(feesPaidOutput) + if err != nil { + return errorsmod.Wrapf(feeshare.ErrFeeSharePayment, "failed to marshal feesPaidOutput: %s", err.Error()) + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + feeshare.EventTypePayoutFeeShare, + sdk.NewAttribute(feeshare.AttributeWithdrawPayouts, string(bz))), + ) + return nil } diff --git a/x/feeshare/keeper/msg_server.go b/x/feeshare/keeper/msg_server.go index 2eab96925..dcd48a1e9 100644 --- a/x/feeshare/keeper/msg_server.go +++ b/x/feeshare/keeper/msg_server.go @@ -16,75 +16,77 @@ import ( var _ types.MsgServer = &Keeper{} -func (k Keeper) GetIfContractWasCreatedFromFactory(ctx sdk.Context, _ 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 - - govAddress := k.accountKeeper.GetModuleAddress(govtypes.ModuleName).String() - - creator, err := sdk.AccAddressFromBech32(info.Creator) - if err != nil { - return false - } - - // If the admin is the gov module (ex: some subDAOs, its a factory contract. Allow register to itself) - if info.Admin == govAddress { +func (k Keeper) GetIfContractWasCreatedFromFactory(ctx sdk.Context, msgSender sdk.AccAddress, info *wasmTypes.ContractInfo) bool { + // Gov Module Admin + govMod := k.accountKeeper.GetModuleAddress(govtypes.ModuleName).String() + if info.Admin == govMod { + // only register to self. return true } - isFactoryContract := false if len(info.Admin) == 0 { - // if there is no admin & the creator is a contract, then its a factory contract - isFactoryContract = k.wasmKeeper.HasContractInfo(ctx, creator) - } else { - admin, err := sdk.AccAddressFromBech32(info.Admin) + // There is no admin. Return if the creator is a contract or normal user. + creator, err := sdk.AccAddressFromBech32(info.Creator) if err != nil { return false } - // if there is an admin & the admin is a contract, then its a factory contract - isFactoryContract = k.wasmKeeper.HasContractInfo(ctx, admin) + + // is factory if creator is a contract. + return k.wasmKeeper.HasContractInfo(ctx, creator) + } + + // There is an admin + admin, err := sdk.AccAddressFromBech32(info.Admin) + if err != nil { + return false + } + + if admin.String() == msgSender.String() { + return false } - return isFactoryContract + return k.wasmKeeper.HasContractInfo(ctx, admin) } // 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 - - // Ensures deployer String is valid + // Ensure the deployer address is valid _, err := sdk.AccAddressFromBech32(deployer) if err != nil { return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid deployer address %s", deployer) } + // Retrieve contract info info := k.wasmKeeper.GetContractInfo(ctx, contract) + // Check if the contract has an admin if len(info.Admin) == 0 { - // no admin, see if they are the creator of the contract + // No admin, so check if the deployer is the creator of the contract if info.Creator != deployer { return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "you are not the creator of this contract %s", info.Creator) } - creatorAddr, err := sdk.AccAddressFromBech32(info.Creator) + _, err := sdk.AccAddressFromBech32(info.Creator) if err != nil { return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address %s", info.Creator) } - controllingAccount = creatorAddr - } else { - // Admin is set, so we check if the deployer is the admin - if info.Admin != deployer { - return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "you are not an admin of this contract %s", deployer) - } - adminAddr, err := sdk.AccAddressFromBech32(info.Admin) - if err != nil { - return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid admin address %s", info.Admin) - } - controllingAccount = adminAddr + // Deployer is the creator, return the controlling account as the creator's address + return sdk.AccAddressFromBech32(info.Creator) + } + + // Admin is set, so check if the deployer is the admin of the contract + if info.Admin != deployer { + return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "you are not an admin of this contract %s", deployer) + } + + // Verify the admin address is valid + _, err = sdk.AccAddressFromBech32(info.Admin) + if err != nil { + return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid admin address %s", info.Admin) } - return controllingAccount, nil + return sdk.AccAddressFromBech32(info.Admin) } // RegisterFeeShare registers a contract to receive transaction fees @@ -116,9 +118,15 @@ func (k Keeper) RegisterFeeShare( return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid withdrawer address %s", msg.WithdrawerAddress) } + // ensure msg.DeployerAddress is valid + msgSender, err := sdk.AccAddressFromBech32(msg.DeployerAddress) + if err != nil { + return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid deployer address %s", msg.DeployerAddress) + } + var deployer sdk.AccAddress - if k.GetIfContractWasCreatedFromFactory(ctx, contract, k.wasmKeeper.GetContractInfo(ctx, contract)) { + if k.GetIfContractWasCreatedFromFactory(ctx, msgSender, 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, errorsmod.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) diff --git a/x/feeshare/keeper/msg_server_test.go b/x/feeshare/keeper/msg_server_test.go index a744f81f0..de9126605 100644 --- a/x/feeshare/keeper/msg_server_test.go +++ b/x/feeshare/keeper/msg_server_test.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/CosmosContracts/juno/v17/x/feeshare/types" ) @@ -113,9 +114,15 @@ func (s *IntegrationTestSuite) TestRegisterFeeShare() { _, _, sender := testdata.KeyTestPubAddr() _ = s.FundAccount(s.ctx, sender, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1_000_000)))) + gov := s.accountKeeper.GetModuleAddress(govtypes.ModuleName).String() + govContract := s.InstantiateContract(sender.String(), gov) + contractAddress := s.InstantiateContract(sender.String(), "") contractAddress2 := s.InstantiateContract(contractAddress, contractAddress) + DAODAO := s.InstantiateContract(sender.String(), "") + subContract := s.InstantiateContract(DAODAO, DAODAO) + _, _, withdrawer := testdata.KeyTestPubAddr() for _, tc := range []struct { @@ -184,6 +191,36 @@ func (s *IntegrationTestSuite) TestRegisterFeeShare() { resp: &types.MsgRegisterFeeShareResponse{}, shouldErr: false, }, + { + desc: "Invalid register gov contract withdraw address", + msg: &types.MsgRegisterFeeShare{ + ContractAddress: govContract, + DeployerAddress: sender.String(), + WithdrawerAddress: sender.String(), + }, + resp: &types.MsgRegisterFeeShareResponse{}, + shouldErr: true, + }, + { + desc: "Success register gov contract withdraw address to self", + msg: &types.MsgRegisterFeeShare{ + ContractAddress: govContract, + DeployerAddress: sender.String(), + WithdrawerAddress: govContract, + }, + resp: &types.MsgRegisterFeeShareResponse{}, + shouldErr: false, + }, + { + desc: "Success register contract from DAODAO contract as admin", + msg: &types.MsgRegisterFeeShare{ + ContractAddress: subContract, + DeployerAddress: DAODAO, + WithdrawerAddress: DAODAO, + }, + resp: &types.MsgRegisterFeeShareResponse{}, + shouldErr: false, + }, } { tc := tc s.Run(tc.desc, func() { diff --git a/x/feeshare/types/events.go b/x/feeshare/types/events.go index a21a7601f..381dd080b 100644 --- a/x/feeshare/types/events.go +++ b/x/feeshare/types/events.go @@ -6,6 +6,9 @@ const ( EventTypeCancelFeeShare = "cancel_feeshare" EventTypeUpdateFeeShare = "update_feeshare" + EventTypePayoutFeeShare = "payout_feeshare" + AttributeKeyContract = "contract" AttributeKeyWithdrawerAddress = "withdrawer_address" + AttributeWithdrawPayouts = "payouts" )