diff --git a/CHANGELOG.md b/CHANGELOG.md index dc062bbff..4d0fa33ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,7 @@ cache if an old BTC delegation receives inclusion proof - [#525](https://github.com/babylonlabs-io/babylon/pull/525) fix: add back `NewIBCHeaderDecorator` post handler - [#563](https://github.com/babylonlabs-io/babylon/pull/563) reject coinbase staking transactions - [#584](https://github.com/babylonlabs-io/babylon/pull/584) fix: Panic can be triggered in handling liveness +- [#585](https://github.com/babylonlabs-io/babylon/pull/585) fix: Proposal vote extensions' byte limit - [#594](https://github.com/babylonlabs-io/babylon/pull/594) Refund tx to correct recipient ## v1.0.0-rc6 diff --git a/app/app.go b/app/app.go index ad37b356c..4898d2800 100644 --- a/app/app.go +++ b/app/app.go @@ -105,7 +105,9 @@ import ( "github.com/babylonlabs-io/babylon/x/btcstkconsumer" bsctypes "github.com/babylonlabs-io/babylon/x/btcstkconsumer/types" "github.com/babylonlabs-io/babylon/x/checkpointing" + "github.com/babylonlabs-io/babylon/x/checkpointing/prepare" checkpointingtypes "github.com/babylonlabs-io/babylon/x/checkpointing/types" + "github.com/babylonlabs-io/babylon/x/checkpointing/vote_extensions" "github.com/babylonlabs-io/babylon/x/epoching" epochingtypes "github.com/babylonlabs-io/babylon/x/epoching/types" "github.com/babylonlabs-io/babylon/x/finality" @@ -501,12 +503,12 @@ func NewBabylonApp( ) // set proposal extension - proposalHandler := checkpointing.NewProposalHandler( + proposalHandler := prepare.NewProposalHandler( logger, &app.CheckpointingKeeper, bApp.Mempool(), bApp, app.EncCfg) proposalHandler.SetHandlers(bApp) // set vote extension - voteExtHandler := checkpointing.NewVoteExtensionHandler(logger, &app.CheckpointingKeeper) + voteExtHandler := vote_extensions.NewVoteExtensionHandler(logger, &app.CheckpointingKeeper) voteExtHandler.SetHandlers(bApp) app.SetInitChainer(app.InitChainer) diff --git a/test/replay/btcstaking_test.go b/test/replay/btcstaking_test.go index 6d80755e4..af448a29c 100644 --- a/test/replay/btcstaking_test.go +++ b/test/replay/btcstaking_test.go @@ -5,9 +5,10 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/babylonlabs-io/babylon/testutil/datagen" bstypes "github.com/babylonlabs-io/babylon/x/btcstaking/types" - "github.com/stretchr/testify/require" ) // TestEpochFinalization checks whether we can finalize some epochs diff --git a/testutil/helper/gen_blocks.go b/testutil/helper/gen_blocks.go index 78af10481..caa145d7f 100644 --- a/testutil/helper/gen_blocks.go +++ b/testutil/helper/gen_blocks.go @@ -140,6 +140,7 @@ func (h *Helper) ApplyEmptyBlockWithVoteExtension(r *rand.Rand) (sdk.Context, er // 3. prepare proposal with previous BLS sigs blockTxs := [][]byte{} ppRes, err := h.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 10000, LocalLastCommit: extendedCommitInfo, Height: newHeight, }) @@ -232,6 +233,7 @@ func (h *Helper) ApplyEmptyBlockWithValSet(r *rand.Rand, valSetWithKeys *datagen // 3. prepare proposal with previous BLS sigs blockTxs := [][]byte{} ppRes, err := h.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 10000, LocalLastCommit: abci.ExtendedCommitInfo{Votes: extendedVotes}, Height: newHeight, }) @@ -333,6 +335,7 @@ func (h *Helper) ApplyEmptyBlockWithInvalidVoteExtensions(r *rand.Rand) (sdk.Con // 3. prepare proposal with previous BLS sigs blockTxs := [][]byte{} ppRes, err := h.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 10000, LocalLastCommit: extendedCommitInfo, Height: newHeight, }) @@ -440,6 +443,7 @@ func (h *Helper) ApplyEmptyBlockWithSomeInvalidVoteExtensions(r *rand.Rand) (sdk var blockTxs [][]byte ppRes, err := h.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 10000, LocalLastCommit: extendedCommitInfo, Height: newHeight, }) diff --git a/x/checkpointing/proposal.go b/x/checkpointing/prepare/proposal.go similarity index 87% rename from x/checkpointing/proposal.go rename to x/checkpointing/prepare/proposal.go index 01d248f22..86fba3842 100644 --- a/x/checkpointing/proposal.go +++ b/x/checkpointing/prepare/proposal.go @@ -1,10 +1,9 @@ -package checkpointing +package prepare import ( "bytes" "encoding/hex" "fmt" - "slices" "cosmossdk.io/log" abci "github.com/cometbft/cometbft/abci/types" @@ -19,15 +18,17 @@ import ( const defaultInjectedTxIndex = 0 +var ( + EmptyProposalRes = abci.ResponsePrepareProposal{Txs: [][]byte{}} +) + type ProposalHandler struct { logger log.Logger ckptKeeper CheckpointingKeeper bApp *baseapp.BaseApp // used for building and parsing the injected tx - txEncoder sdk.TxEncoder - txDecoder sdk.TxDecoder - txBuilder client.TxBuilder + txConfig client.TxConfig defaultPrepareProposalHandler sdk.PrepareProposalHandler defaultProcessProposalHandler sdk.ProcessProposalHandler @@ -47,9 +48,7 @@ func NewProposalHandler( logger: logger, ckptKeeper: ckptKeeper, bApp: bApp, - txEncoder: encCfg.TxConfig.TxEncoder(), - txDecoder: encCfg.TxConfig.TxDecoder(), - txBuilder: encCfg.TxConfig.NewTxBuilder(), + txConfig: encCfg.TxConfig, defaultPrepareProposalHandler: defaultHandler.PrepareProposalHandler(), defaultProcessProposalHandler: defaultHandler.ProcessProposalHandler(), } @@ -74,25 +73,29 @@ func (h *ProposalHandler) PrepareProposal() sdk.PrepareProposalHandler { } k := h.ckptKeeper - proposalTxs := res.Txs - proposalRes := &abci.ResponsePrepareProposal{Txs: proposalTxs} + defaultProposalRes := &abci.ResponsePrepareProposal{Txs: res.Txs} epoch := k.GetEpoch(ctx) // BLS signatures are sent in the last block of the previous epoch, // so they should be aggregated in the first block of the new epoch // and no BLS signatures are send in epoch 0 if !epoch.IsVoteExtensionProposal(ctx) { - return proposalRes, nil + return defaultProposalRes, nil + } + + proposalTxs, err := NewPrepareProposalTxs(req) + if err != nil { + return &EmptyProposalRes, fmt.Errorf("NewPrepareProposalTxs error: %w", err) } if len(req.LocalLastCommit.Votes) == 0 { - return proposalRes, fmt.Errorf("no extended votes received from the last block") + return &EmptyProposalRes, fmt.Errorf("no extended votes received from the last block") } // 1. verify the validity of vote extensions (2/3 majority is achieved) err = baseapp.ValidateVoteExtensions(ctx, h.ckptKeeper, req.Height, ctx.ChainID(), req.LocalLastCommit) if err != nil { - return proposalRes, fmt.Errorf("invalid vote extensions: %w", err) + return &EmptyProposalRes, fmt.Errorf("invalid vote extensions: %w", err) } // 2. build a checkpoint for the previous epoch @@ -100,22 +103,27 @@ func (h *ProposalHandler) PrepareProposal() sdk.PrepareProposalHandler { // we can use the current epoch ckpt, err := h.buildCheckpointFromVoteExtensions(ctx, epoch.EpochNumber, req.LocalLastCommit.Votes) if err != nil { - return proposalRes, fmt.Errorf("failed to build checkpoint from vote extensions: %w", err) + return &EmptyProposalRes, fmt.Errorf("failed to build checkpoint from vote extensions: %w", err) } - // 3. inject a "fake" tx into the proposal s.t. validators can decode, verify the checkpoint - injectedCkpt := &ckpttypes.MsgInjectedCheckpoint{ - Ckpt: ckpt, - ExtendedCommitInfo: &req.LocalLastCommit, + // 3. inject a checkpoint tx into the proposal s.t. validators can decode, verify the checkpoint + injectedVoteExtTx, err := h.buildInjectedTxBytes(ckpt, &req.LocalLastCommit) + if err != nil { + return &EmptyProposalRes, fmt.Errorf("failed to encode vote extensions into a special tx: %w", err) } - injectedVoteExtTx, err := h.buildInjectedTxBytes(injectedCkpt) + + err = proposalTxs.SetOrReplaceCheckpointTx(injectedVoteExtTx) if err != nil { - return nil, fmt.Errorf("failed to encode vote extensions into a special tx: %w", err) + return &EmptyProposalRes, fmt.Errorf("failed to inject checkpoint tx into the proposal: %w", err) + } + + err = proposalTxs.ReplaceOtherTxs(res.Txs) + if err != nil { + return &EmptyProposalRes, fmt.Errorf("failed to add other txs into the proposal: %w", err) } - proposalTxs = slices.Insert(proposalTxs, defaultInjectedTxIndex, [][]byte{injectedVoteExtTx}...) return &abci.ResponsePrepareProposal{ - Txs: proposalTxs, + Txs: proposalTxs.GetTxsInOrder(), }, nil } } @@ -382,12 +390,13 @@ func (h *ProposalHandler) PreBlocker() sdk.PreBlocker { } } -func (h *ProposalHandler) buildInjectedTxBytes(injectedCkpt *ckpttypes.MsgInjectedCheckpoint) ([]byte, error) { - if err := h.txBuilder.SetMsgs(injectedCkpt); err != nil { - return nil, err +func (h *ProposalHandler) buildInjectedTxBytes(ckpt *ckpttypes.RawCheckpointWithMeta, info *abci.ExtendedCommitInfo) ([]byte, error) { + msg := &ckpttypes.MsgInjectedCheckpoint{ + Ckpt: ckpt, + ExtendedCommitInfo: info, } - return h.txEncoder(h.txBuilder.GetTx()) + return EncodeMsgsIntoTxBytes(h.txConfig, msg) } // ExtractInjectedCheckpoint extracts the injected checkpoint from the tx set @@ -402,7 +411,7 @@ func (h *ProposalHandler) ExtractInjectedCheckpoint(txs [][]byte) (*ckpttypes.Ms return nil, fmt.Errorf("the injected vote extensions tx is empty") } - injectedTx, err := h.txDecoder(injectedTxBytes) + injectedTx, err := h.txConfig.TxDecoder()(injectedTxBytes) if err != nil { return nil, fmt.Errorf("failed to decode injected vote extension tx: %w", err) } @@ -425,3 +434,19 @@ func removeInjectedTx(txs [][]byte) ([][]byte, error) { return txs, nil } + +// EncodeMsgsIntoTxBytes encodes the given msgs into a single transaction. +func EncodeMsgsIntoTxBytes(txConfig client.TxConfig, msgs ...sdk.Msg) ([]byte, error) { + txBuilder := txConfig.NewTxBuilder() + err := txBuilder.SetMsgs(msgs...) + if err != nil { + return nil, err + } + + txBytes, err := txConfig.TxEncoder()(txBuilder.GetTx()) + if err != nil { + return nil, err + } + + return txBytes, nil +} diff --git a/x/checkpointing/proposal_expected_keeper.go b/x/checkpointing/prepare/proposal_expected_keeper.go similarity index 97% rename from x/checkpointing/proposal_expected_keeper.go rename to x/checkpointing/prepare/proposal_expected_keeper.go index 697ed90d6..61e8757fc 100644 --- a/x/checkpointing/proposal_expected_keeper.go +++ b/x/checkpointing/prepare/proposal_expected_keeper.go @@ -1,13 +1,14 @@ -package checkpointing +package prepare import ( "context" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/babylonlabs-io/babylon/crypto/bls12381" "github.com/babylonlabs-io/babylon/x/checkpointing/types" epochingtypes "github.com/babylonlabs-io/babylon/x/epoching/types" - cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" - sdk "github.com/cosmos/cosmos-sdk/types" ) type CheckpointingKeeper interface { diff --git a/x/checkpointing/proposal_test.go b/x/checkpointing/prepare/proposal_test.go similarity index 99% rename from x/checkpointing/proposal_test.go rename to x/checkpointing/prepare/proposal_test.go index 65ffb139f..d579fd642 100644 --- a/x/checkpointing/proposal_test.go +++ b/x/checkpointing/prepare/proposal_test.go @@ -1,4 +1,4 @@ -package checkpointing_test +package prepare_test import ( "bytes" @@ -27,7 +27,7 @@ import ( "github.com/babylonlabs-io/babylon/testutil/datagen" "github.com/babylonlabs-io/babylon/testutil/helper" "github.com/babylonlabs-io/babylon/testutil/mocks" - "github.com/babylonlabs-io/babylon/x/checkpointing" + "github.com/babylonlabs-io/babylon/x/checkpointing/prepare" checkpointingtypes "github.com/babylonlabs-io/babylon/x/checkpointing/types" et "github.com/babylonlabs-io/babylon/x/epoching/types" ) @@ -492,7 +492,7 @@ func TestPrepareProposalAtVoteExtensionHeight(t *testing.T) { name := t.Name() encCfg := appparams.DefaultEncodingConfig() bApp := baseapp.NewBaseApp(name, logger, db, encCfg.TxConfig.TxDecoder(), baseapp.SetChainID("chain-test")) - h := checkpointing.NewProposalHandler( + h := prepare.NewProposalHandler( log.NewNopLogger(), ek, mem, diff --git a/x/checkpointing/prepare/transactions.go b/x/checkpointing/prepare/transactions.go new file mode 100644 index 000000000..e49af69f9 --- /dev/null +++ b/x/checkpointing/prepare/transactions.go @@ -0,0 +1,105 @@ +package prepare + +import ( + "errors" + "fmt" + + abci "github.com/cometbft/cometbft/abci/types" +) + +// PrepareProposalTxs is used as an intermediary storage for transactions when creating +// a proposal for `PrepareProposal`. +type PrepareProposalTxs struct { + // Transactions. + CheckpointTx []byte + OtherTxs [][]byte + + // Bytes. + // In general, there's no need to check for int64 overflow given that it would require + // exabytes of memory to hit the max int64 value in bytes. + MaxBytes uint64 + UsedBytes uint64 +} + +// NewPrepareProposalTxs returns a new `PrepareProposalTxs` given the request. +func NewPrepareProposalTxs( + req *abci.RequestPrepareProposal, +) (PrepareProposalTxs, error) { + if req.MaxTxBytes <= 0 { + return PrepareProposalTxs{}, errors.New("MaxTxBytes must be positive") + } + + return PrepareProposalTxs{ + MaxBytes: uint64(req.MaxTxBytes), + UsedBytes: 0, + }, nil +} + +// SetOrReplaceCheckpointTx sets the tx used for checkpoint. If the checkpoint tx already exists, +// replace it +func (t *PrepareProposalTxs) SetOrReplaceCheckpointTx(tx []byte) error { + oldBytes := uint64(len(t.CheckpointTx)) + newBytes := uint64(len(tx)) + if err := t.updateUsedBytes(oldBytes, newBytes); err != nil { + return err + } + t.CheckpointTx = tx + return nil +} + +// ReplaceOtherTxs replaces other txs with the given txs (existing ones are cleared) +func (t *PrepareProposalTxs) ReplaceOtherTxs(allTxs [][]byte) error { + t.OtherTxs = make([][]byte, 0, len(allTxs)) + bytesToAdd := uint64(0) + for _, tx := range allTxs { + txSize := uint64(len(tx)) + if t.UsedBytes+bytesToAdd+txSize > t.MaxBytes { + break + } + + bytesToAdd += txSize + t.OtherTxs = append(t.OtherTxs, tx) + } + + if err := t.updateUsedBytes(0, bytesToAdd); err != nil { + return err + } + + return nil +} + +// updateUsedBytes updates the used bytes field. This returns an error if the num used bytes +// exceeds the max byte limit. +func (t *PrepareProposalTxs) updateUsedBytes( + bytesToRemove uint64, + bytesToAdd uint64, +) error { + if t.UsedBytes < bytesToRemove { + return errors.New("result cannot be negative") + } + + finalBytes := t.UsedBytes - bytesToRemove + bytesToAdd + if finalBytes > t.MaxBytes { + return fmt.Errorf("exceeds max: max=%d, used=%d, adding=%d", t.MaxBytes, t.UsedBytes, bytesToAdd) + } + + t.UsedBytes = finalBytes + return nil +} + +// GetTxsInOrder returns a list of txs in an order that the `ProcessProposal` expects. +func (t *PrepareProposalTxs) GetTxsInOrder() [][]byte { + txsToReturn := make([][]byte, 0, 1+len(t.OtherTxs)) + + // 1. Checkpoint tx + if len(t.CheckpointTx) > 0 { + txsToReturn = append(txsToReturn, t.CheckpointTx) + } + + // 2. "Other" txs + if len(t.OtherTxs) > 0 { + txsToReturn = append(txsToReturn, t.OtherTxs...) + } + + return txsToReturn +} diff --git a/x/checkpointing/prepare/transactions_test.go b/x/checkpointing/prepare/transactions_test.go new file mode 100644 index 000000000..b1464fe06 --- /dev/null +++ b/x/checkpointing/prepare/transactions_test.go @@ -0,0 +1,124 @@ +package prepare_test + +import ( + "testing" + + abci "github.com/cometbft/cometbft/abci/types" + "github.com/stretchr/testify/require" + + "github.com/babylonlabs-io/babylon/x/checkpointing/prepare" +) + +func TestPrepareProposalTxs(t *testing.T) { + t.Run("happy path - can add transactions within limit", func(t *testing.T) { + maxBytes := uint64(100) + req := &abci.RequestPrepareProposal{ + MaxTxBytes: int64(maxBytes), + } + + txs, err := prepare.NewPrepareProposalTxs(req) + require.NoError(t, err) + + // Set checkpoint tx of size 20 + checkpointTx := make([]byte, 20) + err = txs.SetOrReplaceCheckpointTx(checkpointTx) + require.NoError(t, err) + + // Add other txs that fit within remaining space (80 bytes) + otherTxs := [][]byte{ + make([]byte, 30), // tx1 + make([]byte, 30), // tx2 + make([]byte, 10), // tx3 + } + err = txs.ReplaceOtherTxs(otherTxs) + require.NoError(t, err) + + // Verify all txs were added + allTxs := txs.GetTxsInOrder() + require.Equal(t, 4, len(allTxs)) // checkpoint + 3 other txs + require.Equal(t, uint64(90), txs.UsedBytes) + }) + + t.Run("partial addition - some transactions exceed limit", func(t *testing.T) { + maxBytes := uint64(50) + req := &abci.RequestPrepareProposal{ + MaxTxBytes: int64(maxBytes), + } + + txs, err := prepare.NewPrepareProposalTxs(req) + require.NoError(t, err) + + // Set checkpoint tx of size 20 + checkpointTx := make([]byte, 20) + err = txs.SetOrReplaceCheckpointTx(checkpointTx) + require.NoError(t, err) + + // Try to add other txs where some won't fit + otherTxs := [][]byte{ + make([]byte, 20), // tx1 - fits + make([]byte, 20), // tx2 - won't fit + make([]byte, 10), // tx3 - won't fit + } + err = txs.ReplaceOtherTxs(otherTxs) + require.NoError(t, err) + + // Verify only fitting txs were added + allTxs := txs.GetTxsInOrder() + require.Equal(t, 2, len(allTxs)) // checkpoint + tx1 + require.Equal(t, uint64(40), txs.UsedBytes) + }) + + t.Run("full addition - transactions fill space exactly", func(t *testing.T) { + maxBytes := uint64(100) + req := &abci.RequestPrepareProposal{ + MaxTxBytes: int64(maxBytes), + } + + txs, err := prepare.NewPrepareProposalTxs(req) + require.NoError(t, err) + + // Set checkpoint tx of size 40 + checkpointTx := make([]byte, 40) + err = txs.SetOrReplaceCheckpointTx(checkpointTx) + require.NoError(t, err) + + // Add other txs that exactly fill remaining space (60 bytes) + otherTxs := [][]byte{ + make([]byte, 30), // tx1 + make([]byte, 30), // tx2 + } + err = txs.ReplaceOtherTxs(otherTxs) + require.NoError(t, err) + + // Verify all txs were added and space is exactly filled + allTxs := txs.GetTxsInOrder() + require.Equal(t, 3, len(allTxs)) // checkpoint + 2 other txs + require.Equal(t, maxBytes, txs.UsedBytes) + }) + + t.Run("error - checkpoint tx exceeds limit", func(t *testing.T) { + maxBytes := uint64(10) + req := &abci.RequestPrepareProposal{ + MaxTxBytes: int64(maxBytes), + } + + txs, err := prepare.NewPrepareProposalTxs(req) + require.NoError(t, err) + + // Try to set checkpoint tx larger than max + checkpointTx := make([]byte, 20) + err = txs.SetOrReplaceCheckpointTx(checkpointTx) + require.Error(t, err) + require.Contains(t, err.Error(), "exceeds max") + }) + + t.Run("error - negative MaxTxBytes", func(t *testing.T) { + req := &abci.RequestPrepareProposal{ + MaxTxBytes: -1, + } + + _, err := prepare.NewPrepareProposalTxs(req) + require.Error(t, err) + require.Contains(t, err.Error(), "must be positive") + }) +} diff --git a/x/checkpointing/vote_ext.go b/x/checkpointing/vote_extensions/vote_ext.go similarity index 99% rename from x/checkpointing/vote_ext.go rename to x/checkpointing/vote_extensions/vote_ext.go index cbbcdc097..2e0536711 100644 --- a/x/checkpointing/vote_ext.go +++ b/x/checkpointing/vote_extensions/vote_ext.go @@ -1,4 +1,4 @@ -package checkpointing +package vote_extensions import ( "fmt" diff --git a/x/checkpointing/vote_ext_test.go b/x/checkpointing/vote_extensions/vote_ext_test.go similarity index 99% rename from x/checkpointing/vote_ext_test.go rename to x/checkpointing/vote_extensions/vote_ext_test.go index a0001b891..0bff250c3 100644 --- a/x/checkpointing/vote_ext_test.go +++ b/x/checkpointing/vote_extensions/vote_ext_test.go @@ -1,4 +1,4 @@ -package checkpointing_test +package vote_extensions_test import ( "math/rand"