Skip to content

Commit

Permalink
Merge branch 'main' into rp/delay-upgrade-height
Browse files Browse the repository at this point in the history
  • Loading branch information
rootulp authored Jun 18, 2024
2 parents 9b81eb3 + 85eb1cb commit 9184493
Show file tree
Hide file tree
Showing 43 changed files with 1,238 additions and 500 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/create_release_tracking_epic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
types: [released]
jobs:
trigger_issue:
uses: celestiaorg/.github/.github/workflows/[email protected].2
uses: celestiaorg/.github/.github/workflows/[email protected].3
secrets: inherit
with:
release-repo: ${{ github.repository }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/docker-build-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
permissions:
contents: write
packages: write
uses: celestiaorg/.github/.github/workflows/[email protected].2
uses: celestiaorg/.github/.github/workflows/[email protected].3
with:
dockerfile: Dockerfile
secrets: inherit
Expand All @@ -29,7 +29,7 @@ jobs:
permissions:
contents: write
packages: write
uses: celestiaorg/.github/.github/workflows/[email protected].2
uses: celestiaorg/.github/.github/workflows/[email protected].3
with:
dockerfile: docker/Dockerfile_txsim
packageName: txsim
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ jobs:

# hadolint lints the Dockerfile
hadolint:
uses: celestiaorg/.github/.github/workflows/[email protected].2
uses: celestiaorg/.github/.github/workflows/[email protected].3

yamllint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: celestiaorg/.github/.github/actions/[email protected].2
- uses: celestiaorg/.github/.github/actions/[email protected].3
2 changes: 1 addition & 1 deletion .github/workflows/pr-review-requester.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ on:
jobs:
auto-request-review:
name: Auto request reviews
uses: celestiaorg/.github/.github/workflows/[email protected].2
uses: celestiaorg/.github/.github/workflows/[email protected].3
secrets: inherit
# write access for issues and pull requests is needed because the called
# workflow requires write access to issues and pull requests and the
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/test-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,14 @@ jobs:
echo "${KUBECONFIG_FILE}" > $HOME/.kube/config
- name: Run e2e tests
run: make test-e2e

- name: If the e2e test fails, notify Slack channel #e2e-test-failures
if: failure()
uses: ravsamhq/notify-slack-action@v2
with:
status: ${{ job.status }}
token: ${{ secrets.GITHUB_TOKEN }}
notification_title: "E2E test failure"
message_format: "{emoji} *{workflow}* {status_message} in <{run_url}|{repo}>"
env:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_E2E_TEST_FAILURES }}
20 changes: 10 additions & 10 deletions app/ante/fee_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func ValidateTxFeeWrapper(paramKeeper params.Keeper) ante.TxFeeChecker {

// ValidateTxFee implements default fee validation logic for transactions.
// It ensures that the provided transaction fee meets a minimum threshold for the node
// as well as a global minimum threshold and computes the tx priority based on the gas price.
// as well as a network minimum threshold and computes the tx priority based on the gas price.
func ValidateTxFee(ctx sdk.Context, tx sdk.Tx, paramKeeper params.Keeper) (sdk.Coins, int64, error) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
Expand All @@ -50,24 +50,24 @@ func ValidateTxFee(ctx sdk.Context, tx sdk.Tx, paramKeeper params.Keeper) (sdk.C
}
}

// Ensure that the provided fee meets a global minimum threshold.
// Global minimum fee only applies to app versions greater than one
// Ensure that the provided fee meets a network minimum threshold.
// Network minimum fee only applies to app versions greater than one.
if ctx.BlockHeader().Version.App > v1.Version {
subspace, exists := paramKeeper.GetSubspace(minfee.ModuleName)
if !exists {
return nil, 0, errors.Wrap(sdkerror.ErrInvalidRequest, "minfee is not a registered subspace")
}

if !subspace.Has(ctx, minfee.KeyGlobalMinGasPrice) {
return nil, 0, errors.Wrap(sdkerror.ErrKeyNotFound, "GlobalMinGasPrice")
if !subspace.Has(ctx, minfee.KeyNetworkMinGasPrice) {
return nil, 0, errors.Wrap(sdkerror.ErrKeyNotFound, "NetworkMinGasPrice")
}

var globalMinGasPrice sdk.Dec
// Gets the global minimum gas price from the param store
// panics if not configured properly
subspace.Get(ctx, minfee.KeyGlobalMinGasPrice, &globalMinGasPrice)
var networkMinGasPrice sdk.Dec
// Gets the network minimum gas price from the param store.
// Panics if not configured properly.
subspace.Get(ctx, minfee.KeyNetworkMinGasPrice, &networkMinGasPrice)

err := verifyMinFee(fee, gas, globalMinGasPrice, "insufficient gas price for the network")
err := verifyMinFee(fee, gas, networkMinGasPrice, "insufficient gas price for the network")
if err != nil {
return nil, 0, err
}
Expand Down
16 changes: 8 additions & 8 deletions app/ante/min_fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
tmdb "github.com/tendermint/tm-db"
)

func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
func TestValidateTxFee(t *testing.T) {
encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)

builder := encCfg.TxConfig.NewTxBuilder()
Expand Down Expand Up @@ -58,31 +58,31 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
{
name: "bad tx; fee below required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: true,
},
{
name: "good tx; fee equal to required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; fee above required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; with no fee (v1)",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice),
appVersion: uint64(1),
isCheckTx: false,
expErr: false,
Expand Down Expand Up @@ -143,12 +143,12 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {

ctx = ctx.WithMinGasPrices(sdk.DecCoins{validatorMinGasPriceCoin})

globalminGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.GlobalMinGasPrice))
networkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.NetworkMinGasPrice))
require.NoError(t, err)

subspace, _ := paramsKeeper.GetSubspace(minfee.ModuleName)
subspace = minfee.RegisterMinFeeParamTable(subspace)
subspace.Set(ctx, minfee.KeyGlobalMinGasPrice, globalminGasPriceDec)
subspace.Set(ctx, minfee.KeyNetworkMinGasPrice, networkMinGasPriceDec)

_, _, err = ante.ValidateTxFee(ctx, tx, paramsKeeper)
if tc.expErr {
Expand All @@ -173,7 +173,7 @@ func setUp(t *testing.T) (paramkeeper.Keeper, storetypes.CommitMultiStore) {

registry := codectypes.NewInterfaceRegistry()

// Create a params keeper and set the global min gas price
// Create a params keeper and set the network min gas price.
paramsKeeper := paramkeeper.NewKeeper(codec.NewProtoCodec(registry), codec.NewLegacyAmino(), storeKey, tStoreKey)
paramsKeeper.Subspace(minfee.ModuleName)
return paramsKeeper, stateStore
Expand Down
27 changes: 24 additions & 3 deletions app/ante/msg_gatekeeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/authz"
)

var (
Expand All @@ -32,15 +33,35 @@ func (mgk MsgVersioningGateKeeper) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
if !exists {
return ctx, sdkerrors.ErrNotSupported.Wrapf("app version %d is not supported", ctx.BlockHeader().Version.App)
}
for _, msg := range tx.GetMsgs() {

if err := mgk.hasInvalidMsg(ctx, acceptedMsgs, tx.GetMsgs()); err != nil {
return ctx, err
}

return next(ctx, tx, simulate)
}

func (mgk MsgVersioningGateKeeper) hasInvalidMsg(ctx sdk.Context, acceptedMsgs map[string]struct{}, msgs []sdk.Msg) error {
for _, msg := range msgs {
// Recursively check for invalid messages in nested authz messages.
if execMsg, ok := msg.(*authz.MsgExec); ok {
nestedMsgs, err := execMsg.GetMessages()
if err != nil {
return err
}
if err = mgk.hasInvalidMsg(ctx, acceptedMsgs, nestedMsgs); err != nil {
return err
}
}

msgTypeURL := sdk.MsgTypeURL(msg)
_, exists := acceptedMsgs[msgTypeURL]
if !exists {
return ctx, sdkerrors.ErrNotSupported.Wrapf("message type %s is not supported in version %d", msgTypeURL, ctx.BlockHeader().Version.App)
return sdkerrors.ErrNotSupported.Wrapf("message type %s is not supported in version %d", msgTypeURL, ctx.BlockHeader().Version.App)
}
}

return next(ctx, tx, simulate)
return nil
}

func (mgk MsgVersioningGateKeeper) IsAllowed(ctx context.Context, msgName string) (bool, error) {
Expand Down
39 changes: 37 additions & 2 deletions app/ante/msg_gatekeeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ import (
"github.com/celestiaorg/celestia-app/v2/app/ante"
"github.com/celestiaorg/celestia-app/v2/app/encoding"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/authz"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
version "github.com/tendermint/tendermint/proto/tendermint/version"
)

func TestMsgGateKeeperAnteHandler(t *testing.T) {
nestedBankSend := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&banktypes.MsgSend{}})
nestedMultiSend := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&banktypes.MsgMultiSend{}})

// Define test cases
tests := []struct {
name string
Expand All @@ -27,23 +31,42 @@ func TestMsgGateKeeperAnteHandler(t *testing.T) {
acceptMsg: true,
version: 1,
},
{
name: "Accept nested MsgSend",
msg: &nestedBankSend,
acceptMsg: true,
version: 1,
},
{
name: "Reject MsgMultiSend",
msg: &banktypes.MsgMultiSend{},
acceptMsg: false,
version: 1,
},
{
name: "Reject nested MsgMultiSend",
msg: &nestedMultiSend,
acceptMsg: false,
version: 1,
},
{
name: "Reject MsgSend with version 2",
msg: &banktypes.MsgSend{},
acceptMsg: false,
version: 2,
},
{
name: "Reject nested MsgSend with version 2",
msg: &nestedBankSend,
acceptMsg: false,
version: 2,
},
}

msgGateKeeper := ante.NewMsgVersioningGateKeeper(map[uint64]map[string]struct{}{
1: {
"/cosmos.bank.v1beta1.MsgSend": {},
"/cosmos.bank.v1beta1.MsgSend": {},
"/cosmos.authz.v1beta1.MsgExec": {},
},
2: {},
})
Expand All @@ -56,7 +79,19 @@ func TestMsgGateKeeperAnteHandler(t *testing.T) {
txBuilder := cdc.TxConfig.NewTxBuilder()
require.NoError(t, txBuilder.SetMsgs(tc.msg))
_, err := anteHandler(ctx, txBuilder.GetTx(), false)
allowed, err2 := msgGateKeeper.IsAllowed(ctx, sdk.MsgTypeURL(tc.msg))

msg := tc.msg
if sdk.MsgTypeURL(msg) == "/cosmos.authz.v1beta1.MsgExec" {
execMsg, ok := msg.(*authz.MsgExec)
require.True(t, ok)

nestedMsgs, err := execMsg.GetMessages()
require.NoError(t, err)
msg = nestedMsgs[0]
}

allowed, err2 := msgGateKeeper.IsAllowed(ctx, sdk.MsgTypeURL(msg))

require.NoError(t, err2)
if tc.acceptMsg {
require.NoError(t, err, "expected message to be accepted")
Expand Down
10 changes: 5 additions & 5 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,13 +301,13 @@ func New(

paramBlockList := paramfilter.NewParamBlockList(app.BlockedParams()...)

// register the proposal types
// Register the proposal types.
govRouter := oldgovtypes.NewRouter()
govRouter.AddRoute(paramproposal.RouterKey, paramBlockList.GovHandler(app.ParamsKeeper)).
AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
AddRoute(ibcclienttypes.RouterKey, NewClientProposalHandler(app.IBCKeeper.ClientKeeper))

// Create Transfer Keepers
// Create Transfer Keepers.
tokenFilterKeeper := tokenfilter.NewKeeper(app.IBCKeeper.ChannelKeeper)

app.PacketForwardKeeper = packetforwardkeeper.NewKeeper(
Expand All @@ -326,7 +326,7 @@ func New(
app.PacketForwardKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, app.BankKeeper, app.ScopedTransferKeeper,
)
// transfer stack contains (from top to bottom):
// Transfer stack contains (from top to bottom):
// - Token Filter
// - Packet Forwarding Middleware
// - Transfer
Expand All @@ -339,9 +339,9 @@ func New(
packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp, // forward timeout
packetforwardkeeper.DefaultRefundTransferPacketTimeoutTimestamp, // refund timeout
)
// packetForwardMiddleware is used only for version 2
// PacketForwardMiddleware is used only for version 2.
transferStack = module.NewVersionedIBCModule(packetForwardMiddleware, transferStack, v2, v2)
// token filter wraps packet forward middleware and is thus the first module in the transfer stack
// Token filter wraps packet forward middleware and is thus the first module in the transfer stack.
tokenFilterMiddelware := tokenfilter.NewIBCMiddleware(transferStack)
transferStack = module.NewVersionedIBCModule(tokenFilterMiddelware, transferStack, v1, v2)

Expand Down
6 changes: 6 additions & 0 deletions app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/celestiaorg/celestia-app/v2/app"
"github.com/celestiaorg/celestia-app/v2/app/encoding"
"github.com/celestiaorg/celestia-app/v2/x/minfee"
"github.com/stretchr/testify/assert"
"github.com/tendermint/tendermint/libs/log"
tmdb "github.com/tendermint/tm-db"
Expand Down Expand Up @@ -39,6 +40,11 @@ func TestNew(t *testing.T) {
t.Run("should not have sealed the baseapp", func(t *testing.T) {
assert.False(t, got.IsSealed())
})
t.Run("should have set the minfee key table", func(t *testing.T) {
subspace := got.GetSubspace(minfee.ModuleName)
hasKeyTable := subspace.HasKeyTable()
assert.True(t, hasKeyTable)
})
}

// NoopWriter is a no-op implementation of a writer.
Expand Down
6 changes: 3 additions & 3 deletions app/test/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
// TestAppUpgrades verifies that the all module's params are overridden during an
// upgrade from v1 -> v2 and the app version changes correctly.
func TestAppUpgrades(t *testing.T) {
GlobalMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.GlobalMinGasPrice))
NetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.NetworkMinGasPrice))
require.NoError(t, err)

tests := []struct {
Expand All @@ -42,8 +42,8 @@ func TestAppUpgrades(t *testing.T) {
{
module: "MinFee",
subspace: minfee.ModuleName,
key: string(minfee.KeyGlobalMinGasPrice),
expectedValue: GlobalMinGasPriceDec.String(),
key: string(minfee.KeyNetworkMinGasPrice),
expectedValue: NetworkMinGasPriceDec.String(),
},
{
module: "ICA",
Expand Down
Loading

0 comments on commit 9184493

Please sign in to comment.