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.

Sign up for GitHub

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

Jump to bottom

feat: consensus messages #141

Merged
merged 9 commits into from
Oct 21, 2024
Merged

feat: consensus messages #141

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3ba107d
add changes to use the consensus messages
Sep 27, 2024
d33f49c
goimports
Oct 4, 2024
d188a3d
use banktypes
Oct 4, 2024
418b9b5
goimports
Oct 4, 2024
d9429a2
Merge remote-tracking branch 'origin/main' into feat/consensus-messages
Oct 4, 2024
3a4370f
use process consensus messages from rdk
Oct 9, 2024
e74854f
Merge remote-tracking branch 'origin/main' into feat/consensus-messages
Oct 20, 2024
7cb61d7
use last cometbft
Oct 20, 2024
7760b3e
use also real rdk
Oct 20, 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
91 changes: 89 additions & 2 deletions app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app

import (
"fmt"
"github.com/dymensionxyz/dymension-rdk/server/consensus"
faultytolly marked this conversation as resolved.
Show resolved Hide resolved
"io"
"net/http"
"os"
Expand All @@ -14,6 +15,8 @@ import (
gaslessmodule "github.com/dymensionxyz/dymension-rdk/x/gasless"
gaslesskeeper "github.com/dymensionxyz/dymension-rdk/x/gasless/keeper"
gaslesstypes "github.com/dymensionxyz/dymension-rdk/x/gasless/types"
"github.com/gogo/protobuf/proto"
prototypes "github.com/gogo/protobuf/types"
"github.com/gorilla/mux"
"github.com/rakyll/statik/fs"
abci "github.com/tendermint/tendermint/abci/types"
Expand Down Expand Up @@ -310,6 +313,8 @@ type App struct {

// module configurator
configurator module.Configurator

consensusMessageAdmissionHandler consensus.AdmissionHandler
}

// NewRollapp returns a reference to an initialized blockchain app
Expand Down Expand Up @@ -643,7 +648,7 @@ func NewRollapp(
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, app.BankKeeper),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
sequencers.NewAppModule(appCodec, app.SequencersKeeper),
sequencers.NewAppModule(app.SequencersKeeper),
epochs.NewAppModule(appCodec, app.EpochsKeeper),
params.NewAppModule(app.ParamsKeeper),
wasm.NewAppModule(appCodec, &app.WasmKeeper, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
Expand Down Expand Up @@ -810,6 +815,13 @@ func NewRollapp(
// upgrade.
app.setPostHandler()

// Admission handler for consensus messages
app.setAdmissionHandler(consensus.MapAdmissionHandler(
[]string{
proto.MessageName(&banktypes.MsgSend{}),
},
))
faultytolly marked this conversation as resolved.
Show resolved Hide resolved

if loadLatest {
if err := app.LoadLatestVersion(); err != nil {
tmos.Exit(err.Error())
Expand Down Expand Up @@ -863,12 +875,87 @@ func (app *App) setPostHandler() {
app.SetPostHandler(postHandler)
}

func (app *App) setAdmissionHandler(handler consensus.AdmissionHandler) {
app.consensusMessageAdmissionHandler = handler
}

// Name returns the name of the App
func (app *App) Name() string { return app.BaseApp.Name() }

// BeginBlocker application updates every begin block
func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
return app.mm.BeginBlock(ctx, req)
consensusResponses := app.processConsensusMessage(ctx, req.ConsensusMessages)

resp := app.mm.BeginBlock(ctx, req)
resp.ConsensusMessagesResponses = consensusResponses

return resp
}

func (app *App) processConsensusMessage(ctx sdk.Context, consensusMsgs []*prototypes.Any) []*abci.ConsensusMessageResponse {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments as on the evm repo - and also as I wrote there - I suggest moving this method to common util on the RDK vs re-writing it (if possible) .

var responses []*abci.ConsensusMessageResponse

for _, anyMsg := range consensusMsgs {
sdkAny := &types.Any{
TypeUrl: "/" + anyMsg.TypeUrl,
Value: anyMsg.Value,
}

var msg sdk.Msg
err := app.appCodec.UnpackAny(sdkAny, &msg)
if err != nil {
responses = append(responses, &abci.ConsensusMessageResponse{
Response: &abci.ConsensusMessageResponse_Error{
Error: fmt.Errorf("failed to unpack consensus message: %w", err).Error(),
},
})

continue
}

cacheCtx, writeCache := ctx.CacheContext()
err = app.consensusMessageAdmissionHandler(cacheCtx, msg)
if err != nil {
responses = append(responses, &abci.ConsensusMessageResponse{
Response: &abci.ConsensusMessageResponse_Error{
Error: fmt.Errorf("consensus message admission failed: %w", err).Error(),
},
})

continue
}

resp, err := app.MsgServiceRouter().Handler(msg)(ctx, msg)
if err != nil {
responses = append(responses, &abci.ConsensusMessageResponse{
Response: &abci.ConsensusMessageResponse_Error{
Error: fmt.Errorf("failed to execute consensus message: %w", err).Error(),
},
})

continue
}

theType, err := proto.Marshal(resp)
if err != nil {
return nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return nil? don't you need to append to responses and continue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to verify if this type of error is expected

}

anyResp := &prototypes.Any{
TypeUrl: proto.MessageName(resp),
Value: theType,
}

responses = append(responses, &abci.ConsensusMessageResponse{
Response: &abci.ConsensusMessageResponse_Ok{
Ok: anyResp,
},
})

writeCache()
}

return responses
}

// EndBlocker application updates every end block
Expand Down
100 changes: 100 additions & 0 deletions app/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package app

import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
types2 "github.com/cosmos/cosmos-sdk/x/bank/types"
faultytolly marked this conversation as resolved.
Show resolved Hide resolved
"github.com/gogo/protobuf/proto"
prototypes "github.com/gogo/protobuf/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/proto/tendermint/types"
)

func TestBeginBlocker(t *testing.T) {
app, valAccount := SetupWithOneValidator(t)
ctx := app.NewUncachedContext(true, types.Header{
Height: 1,
ChainID: "testchain_9000-1",
})

bankSend := &types2.MsgSend{
FromAddress: valAccount.GetAddress().String(),
ToAddress: valAccount.GetAddress().String(),
Amount: sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(100))),
}
msgBz, err := proto.Marshal(bankSend)
require.NoError(t, err)

goodMessage := &prototypes.Any{
TypeUrl: proto.MessageName(&types2.MsgSend{}),
Value: msgBz,
}

testCases := []struct {
name string
consensusMsgs []*prototypes.Any
expectError bool
}{
{
name: "ValidConsensusMessage",
consensusMsgs: []*prototypes.Any{
goodMessage,
},
expectError: false,
},
{
name: "InvalidUnpackMessage",
consensusMsgs: []*prototypes.Any{
{
TypeUrl: "/path.to.InvalidMsg",
Value: []byte("invalid unpack data"),
},
},
expectError: true,
},
{
name: "InvalidExecutionMessage",
consensusMsgs: []*prototypes.Any{
{
TypeUrl: "/path.to.ExecErrorMsg",
Value: []byte("execution error data"),
},
},
expectError: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
req := abci.RequestBeginBlock{
Header: types.Header{
Height: 1,
Time: ctx.BlockTime(),
ChainID: "testchain_9000-1",
},
LastCommitInfo: abci.LastCommitInfo{},
ByzantineValidators: []abci.Evidence{},
ConsensusMessages: tc.consensusMsgs,
}

res := app.BeginBlocker(ctx, req)
require.NotNil(t, res)

if tc.expectError {
require.NotEmpty(t, res.ConsensusMessagesResponses)
for _, response := range res.ConsensusMessagesResponses {
_, isError := response.Response.(*abci.ConsensusMessageResponse_Error)
require.True(t, isError, "Expected an error response but got a success")
Comment on lines +88 to +89
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd also suggest verifying the returned error string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danwt does not like to check error strings

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea but it's still better than nothing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think in theory you can check grpc status code
i haven't looked at it tho

}
} else {
require.NotEmpty(t, res.ConsensusMessagesResponses)
for _, response := range res.ConsensusMessagesResponses {
_, isOk := response.Response.(*abci.ConsensusMessageResponse_Ok)
require.True(t, isOk, "Expected a success response but got an error")
}
}
})
}
}
Loading
Loading