Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Whitelist tokenfactory hooks #587

Merged
merged 26 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e855c8f
basic whitelist logic
jcompagni10 Jun 19, 2024
2251fdd
Fix tests
jcompagni10 Jun 19, 2024
c07f823
Add note on weird TF code
jcompagni10 Jun 19, 2024
209c6b3
Block adding non-whitelisted hooks
jcompagni10 Jun 19, 2024
1b2cdec
Refactor + add more tests
jcompagni10 Jun 19, 2024
5469b79
add migration
jcompagni10 Jun 19, 2024
448149e
lint
jcompagni10 Jun 19, 2024
09af904
Merge remote-tracking branch 'origin/main' into feat/whitelist-tf-hooks
jcompagni10 Jun 19, 2024
5b971b1
add migration to remove non-whitelisted tf hooks
jcompagni10 Jun 19, 2024
d4b551c
lint
jcompagni10 Jun 19, 2024
71ee07e
misc review fixes
jcompagni10 Jun 19, 2024
a6eae66
regen swagger
jcompagni10 Jun 19, 2024
b5d02d4
PR review fixes
jcompagni10 Jun 20, 2024
fff1228
Merge remote-tracking branch 'origin/main' into feat/whitelist-tf-hooks
jcompagni10 Jun 20, 2024
01622b6
small migration fixes
jcompagni10 Jun 20, 2024
33b2846
fix: dont write during iterations
jcompagni10 Jun 20, 2024
024ce3d
lint
jcompagni10 Jun 20, 2024
36fd28a
Add error logging for skipped hooks
jcompagni10 Jun 20, 2024
2c7fb7d
Add whitelisted astroport contracts to TF migration
jcompagni10 Jun 20, 2024
01c1923
Add missing comma in init-neutrond.sh
jcompagni10 Jun 20, 2024
a097ad9
better comments on TF params.proto
jcompagni10 Jun 21, 2024
db7c03d
Merge branch 'main' into feat/whitelist-tf-hooks
pr0n00gler Jun 21, 2024
daf306d
make errors clearer
jcompagni10 Jun 21, 2024
9077e49
fixes to tokenfactory query cli
jcompagni10 Jun 21, 2024
32bf31b
update code_ids for astroport contract whitelisting
jcompagni10 Jun 21, 2024
3e26ebb
do not use a separate event manager for sudo calls in tokenfactory
pr0n00gler Jun 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 89 additions & 11 deletions docs/static/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18637,17 +18637,7 @@ definitions:
type: array
type: object
type: object
osmosis.tokenfactory.v1beta1.DenomAuthorityMetadata:
description: |-
DenomAuthorityMetadata specifies metadata for addresses that have specific
capabilities over a token factory denom. Right now there is only one Admin
permission, but is planned to be extended to the future.
properties:
Admin:
title: Can be empty for no admin, or a valid osmosis address
type: string
type: object
osmosis.tokenfactory.v1beta1.Params:
osmosis.tokenfactory.Params:
description: Params defines the parameters for the tokenfactory module.
properties:
denom_creation_fee:
Expand Down Expand Up @@ -18686,6 +18676,52 @@ definitions:

are sent to
type: string
whitelisted_hooks:
items:
properties:
code_id:
format: uint64
type: string
denom_creator:
type: string
title: >-
WhitelistedHook describes a beforeSendHook which is allowed to be
added and executed

SetBeforeSendHook can only be called on denoms where the denom
creator and

code_id for the `contract_addr` match a WhitelistedHook
type: object
title: >-
whitelisted_hooks is the list of hooks which are allowed to be added
and executed
type: array
type: object
osmosis.tokenfactory.WhitelistedHook:
properties:
code_id:
format: uint64
type: string
denom_creator:
type: string
title: >-
WhitelistedHook describes a beforeSendHook which is allowed to be added
and executed

SetBeforeSendHook can only be called on denoms where the denom creator and

code_id for the `contract_addr` match a WhitelistedHook
type: object
osmosis.tokenfactory.v1beta1.DenomAuthorityMetadata:
description: |-
DenomAuthorityMetadata specifies metadata for addresses that have specific
capabilities over a token factory denom. Right now there is only one Admin
permission, but is planned to be extended to the future.
properties:
Admin:
title: Can be empty for no admin, or a valid osmosis address
type: string
type: object
osmosis.tokenfactory.v1beta1.QueryBeforeSendHookAddressResponse:
description: |-
Expand Down Expand Up @@ -18774,6 +18810,27 @@ definitions:

are sent to
type: string
whitelisted_hooks:
items:
properties:
code_id:
format: uint64
type: string
denom_creator:
type: string
title: >-
WhitelistedHook describes a beforeSendHook which is allowed to
be added and executed

SetBeforeSendHook can only be called on denoms where the denom
creator and

code_id for the `contract_addr` match a WhitelistedHook
type: object
title: >-
whitelisted_hooks is the list of hooks which are allowed to be
added and executed
type: array
type: object
type: object
packetforward.v1.Params:
Expand Down Expand Up @@ -43742,6 +43799,27 @@ paths:

are sent to
type: string
whitelisted_hooks:
items:
properties:
code_id:
format: uint64
type: string
denom_creator:
type: string
title: >-
WhitelistedHook describes a beforeSendHook which is
allowed to be added and executed

SetBeforeSendHook can only be called on denoms where the
denom creator and

code_id for the `contract_addr` match a WhitelistedHook
type: object
title: >-
whitelisted_hooks is the list of hooks which are allowed
to be added and executed
type: array
type: object
type: object
default:
Expand Down
2 changes: 1 addition & 1 deletion network/init-neutrond.sh
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ echo $DAO_CONTRACT_ADDRESS_B64

set_genesis_param admins "[\"$NEUTRON_CHAIN_MANAGER_CONTRACT_ADDRESS\"]" # admin module
set_genesis_param treasury_address "\"$DAO_CONTRACT_ADDRESS\"" # feeburner
set_genesis_param fee_collector_address "\"$DAO_CONTRACT_ADDRESS\"" # tokenfactory
set_genesis_param fee_collector_address "\"$DAO_CONTRACT_ADDRESS\"," # tokenfactory
set_genesis_param security_address "\"$SECURITY_SUBDAO_CORE_CONTRACT_ADDRESS\"," # cron
set_genesis_param limit 5 # cron
#set_genesis_param allow_messages "[\"*\"]" # interchainaccounts
Expand Down
43 changes: 43 additions & 0 deletions proto/osmosis/tokenfactory/params.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
syntax = "proto3";
package osmosis.tokenfactory;

import "cosmos/base/v1beta1/coin.proto";
import "cosmos_proto/cosmos.proto";
import "gogoproto/gogo.proto";

option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";

// WhitelistedHook describes a beforeSendHook which is allowed to be added and executed
// SetBeforeSendHook can only be called on denoms where the denom creator and
// code_id for the `contract_addr` match a WhitelistedHook
message WhitelistedHook {
uint64 code_id = 1 [(gogoproto.customname) = "CodeID"];
string denom_creator = 2;
}

// Params defines the parameters for the tokenfactory module.
message Params {
// DenomCreationFee defines the fee to be charged on the creation of a new
// denom. The fee is drawn from the MsgCreateDenom's sender account, and
// transferred to the community pool.
repeated cosmos.base.v1beta1.Coin denom_creation_fee = 1 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
(gogoproto.moretags) = "yaml:\"denom_creation_fee\"",
(gogoproto.nullable) = false
];

// DenomCreationGasConsume defines the gas cost for creating a new denom.
// This is intended as a spam deterrence mechanism.
//
// See: https://github.com/CosmWasm/token-factory/issues/11
uint64 denom_creation_gas_consume = 2 [
(gogoproto.moretags) = "yaml:\"denom_creation_gas_consume\"",
(gogoproto.nullable) = true
];

// FeeCollectorAddress is the address where fees collected from denom creation
// are sent to
string fee_collector_address = 3;
// whitelisted_hooks is the list of hooks which are allowed to be added and executed
repeated WhitelistedHook whitelisted_hooks = 4;
}
2 changes: 1 addition & 1 deletion proto/osmosis/tokenfactory/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ syntax = "proto3";
package osmosis.tokenfactory.v1beta1;

import "gogoproto/gogo.proto";
import "osmosis/tokenfactory/params.proto";
import "osmosis/tokenfactory/v1beta1/authorityMetadata.proto";
import "osmosis/tokenfactory/v1beta1/params.proto";

option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";

Expand Down
2 changes: 1 addition & 1 deletion proto/osmosis/tokenfactory/v1beta1/params.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "cosmos/base/v1beta1/coin.proto";
import "cosmos_proto/cosmos.proto";
import "gogoproto/gogo.proto";

option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types/v1beta1";

// Params defines the parameters for the tokenfactory module.
message Params {
Expand Down
2 changes: 1 addition & 1 deletion proto/osmosis/tokenfactory/v1beta1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package osmosis.tokenfactory.v1beta1;

import "gogoproto/gogo.proto";
import "google/api/annotations.proto";
import "osmosis/tokenfactory/params.proto";
import "osmosis/tokenfactory/v1beta1/authorityMetadata.proto";
import "osmosis/tokenfactory/v1beta1/params.proto";

option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";

Expand Down
2 changes: 1 addition & 1 deletion proto/osmosis/tokenfactory/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "cosmos/base/v1beta1/coin.proto";
import "cosmos/msg/v1/msg.proto";
import "cosmos_proto/cosmos.proto";
import "gogoproto/gogo.proto";
import "osmosis/tokenfactory/v1beta1/params.proto";
import "osmosis/tokenfactory/params.proto";

option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";

Expand Down
1 change: 1 addition & 0 deletions wasmbinding/test/custom_message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func (suite *CustomMessengerTestSuite) SetupTest() {
sdk.NewCoins(sdk.NewInt64Coin(params.DefaultDenom, 10_000_000)),
0,
FeeCollectorAddress,
tokenfactorytypes.DefaultWhitelistedHooks,
))
suite.Require().NoError(err)

Expand Down
1 change: 1 addition & 0 deletions wasmbinding/test/custom_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ func (suite *CustomQuerierTestSuite) TestDenomAdmin() {
sdk.NewCoins(sdk.NewInt64Coin(params.DefaultDenom, 10_000_000)),
0,
FeeCollectorAddress,
tokenfactorytypes.DefaultWhitelistedHooks,
))
suite.Require().NoError(err)

Expand Down
35 changes: 35 additions & 0 deletions x/tokenfactory/keeper/before_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@ func (k Keeper) Hooks() Hooks {
return Hooks{k}
}

func (k Keeper) AssertIsHookWhitelisted(ctx sdk.Context, denom string, contractAddress sdk.AccAddress) error {
contractInfo := k.contractKeeper.GetContractInfo(ctx, contractAddress)
if contractInfo == nil {
return types.ErrBeforeSendHookNotWhitelisted.Wrapf("contract with address (%s) does not exist", contractAddress.String())
}
codeID := contractInfo.CodeID
whitelistedHooks := k.GetParams(ctx).WhitelistedHooks
denomCreator, _, err := types.DeconstructDenom(denom)
if err != nil {
return types.ErrBeforeSendHookNotWhitelisted.Wrapf("invalid denom: %s", denom)
}

for _, hook := range whitelistedHooks {
if hook.CodeID == codeID && hook.DenomCreator == denomCreator {
return nil
}
}

return types.ErrBeforeSendHookNotWhitelisted.Wrapf("no whitelist for contract with codeID (%d) and denomCreator (%s) ", codeID, denomCreator)
}

// TrackBeforeSend calls the before send listener contract suppresses any errors
func (h Hooks) TrackBeforeSend(ctx context.Context, from, to sdk.AccAddress, amount sdk.Coins) {
_ = h.k.callBeforeSendListener(ctx, from, to, amount, false)
Expand Down Expand Up @@ -99,6 +120,20 @@ func (k Keeper) callBeforeSendListener(ctx context.Context, from, to sdk.AccAddr
return err
}

// Do not invoke hook if denom is not whitelisted
// NOTE: hooks must already be whitelisted before they can be added, so under normal operation this check should never fail.
// It is here as an emergency override if we want to shutoff a hook. We do not return the error because once it is removed from the whitelist
// a hook should not be able to block a send.
if err := k.AssertIsHookWhitelisted(c, coin.Denom, cwAddr); err != nil {
c.Logger().Error(
"Skipped hook execution due to missing whitelist",
"err", err,
"denom", coin.Amount,
sotnikov-s marked this conversation as resolved.
Show resolved Hide resolved
"contract", cwAddr.String(),
)
continue
}

var msgBz []byte

// get msgBz, either BlockBeforeSend or TrackBeforeSend
Expand Down
Loading
Loading