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

fix: Switch from stdlib json encoding to proto encoding in vote extension handler #516

Merged
merged 9 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions app/preblocker.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package app

import (
"encoding/json"
"fmt"

"cosmossdk.io/core/appmodule"
cometabci "github.com/cometbft/cometbft/abci/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ojo-network/ojo/x/oracle/abci"
"github.com/ojo-network/ojo/x/oracle/types"
)

// PreBlocker is run before finalize block to update the aggregrate exchange rate votes on the oracle module
Expand Down Expand Up @@ -45,8 +45,8 @@ func (app *App) PreBlocker(ctx sdk.Context, req *cometabci.RequestFinalizeBlock)
}
voteExtensionsEnabled := abci.VoteExtensionsEnabled(ctx)
if voteExtensionsEnabled {
var injectedVoteExtTx abci.AggregateExchangeRateVotes
if err := json.Unmarshal(req.Txs[0], &injectedVoteExtTx); err != nil {
var injectedVoteExtTx types.InjectedVoteExtensionTx
if err := injectedVoteExtTx.Unmarshal(req.Txs[0]); err != nil {
app.Logger().Error("failed to decode injected vote extension tx", "err", err)
return nil, err
}
Expand Down
29 changes: 29 additions & 0 deletions proto/ojo/oracle/v1/abci.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
syntax = "proto3";
package ojo.oracle.v1;

import "gogoproto/gogo.proto";
import "cosmos/base/v1beta1/coin.proto";
import "ojo/oracle/v1/oracle.proto";

option go_package = "github.com/ojo-network/ojo/x/oracle/types";

option (gogoproto.goproto_getters_all) = false;

// OracleVoteExtension defines the vote extension structure used by the oracle
// module.
message OracleVoteExtension {
int64 height = 1;
repeated cosmos.base.v1beta1.DecCoin exchange_rates = 2 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
}

// InjectedVoteExtensionTx defines the vote extension tx injected by the prepare
// proposal handler.
message InjectedVoteExtensionTx {
repeated AggregateExchangeRateVote exchange_rate_votes = 1[
(gogoproto.nullable) = false
];
bytes extended_commit_info = 2;
}
2 changes: 1 addition & 1 deletion x/oracle/abci/endblocker.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func EndBlocker(ctx context.Context, k keeper.Keeper) error {

// Execute price feeder oracle tick.
if err := k.PriceFeeder.Oracle.TickClientless(ctx); err != nil {
return err
sdkCtx.Logger().Error("Error in Oracle Keeper price feeder clientless tick", "err", err)
}
}

Expand Down
42 changes: 25 additions & 17 deletions x/oracle/abci/proposal.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package abci

import (
"encoding/json"
"fmt"
"sort"

Expand All @@ -16,11 +15,6 @@ import (
oracletypes "github.com/ojo-network/ojo/x/oracle/types"
)

type AggregateExchangeRateVotes struct {
ExchangeRateVotes []oracletypes.AggregateExchangeRateVote
ExtendedCommitInfo cometabci.ExtendedCommitInfo
}

type ProposalHandler struct {
logger log.Logger
oracleKeeper oraclekeeper.Keeper
Expand Down Expand Up @@ -74,15 +68,17 @@ func (h *ProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler {
if err != nil {
return &cometabci.ResponsePrepareProposal{Txs: make([][]byte, 0)}, err
}
extendedCommitInfoBz, err := req.LocalLastCommit.Marshal()
Fixed Show fixed Hide fixed
if err != nil {
return &cometabci.ResponsePrepareProposal{Txs: make([][]byte, 0)}, err
}

injectedVoteExtTx := AggregateExchangeRateVotes{
injectedVoteExtTx := oracletypes.InjectedVoteExtensionTx{
ExchangeRateVotes: exchangeRateVotes,
ExtendedCommitInfo: req.LocalLastCommit,
ExtendedCommitInfo: extendedCommitInfoBz,
}

// TODO: Switch from stdlib JSON encoding to a more performant mechanism.
// REF: https://github.com/ojo-network/ojo/issues/411
bz, err := json.Marshal(injectedVoteExtTx)
bz, err := injectedVoteExtTx.Marshal()
if err != nil {
h.logger.Error("failed to encode injected vote extension tx", "err", err)
return &cometabci.ResponsePrepareProposal{Txs: make([][]byte, 0)}, oracletypes.ErrEncodeInjVoteExt
Expand Down Expand Up @@ -127,25 +123,37 @@ func (h *ProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler {

voteExtensionsEnabled := VoteExtensionsEnabled(ctx)
if voteExtensionsEnabled {
var injectedVoteExtTx AggregateExchangeRateVotes
if err := json.Unmarshal(req.Txs[0], &injectedVoteExtTx); err != nil {
if len(req.Txs) < 1 {
h.logger.Error("got process proposal request with no commit info")
return &cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT},
oracletypes.ErrNoCommitInfo
}

var injectedVoteExtTx oracletypes.InjectedVoteExtensionTx
if err := injectedVoteExtTx.Unmarshal(req.Txs[0]); err != nil {
h.logger.Error("failed to decode injected vote extension tx", "err", err)
return &cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}, err
}
var extendedCommitInfo cometabci.ExtendedCommitInfo
if err := extendedCommitInfo.Unmarshal(injectedVoteExtTx.ExtendedCommitInfo); err != nil {
h.logger.Error("failed to decode injected extended commit info", "err", err)
return &cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}, err
}

err := baseapp.ValidateVoteExtensions(
ctx,
h.stakingKeeper,
req.Height,
ctx.ChainID(),
injectedVoteExtTx.ExtendedCommitInfo,
extendedCommitInfo,
)
if err != nil {
return &cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}, err
}

// Verify the proposer's oracle exchange rate votes by computing the same
// calculation and comparing the results.
exchangeRateVotes, err := h.generateExchangeRateVotes(ctx, injectedVoteExtTx.ExtendedCommitInfo)
exchangeRateVotes, err := h.generateExchangeRateVotes(ctx, extendedCommitInfo)
if err != nil {
return &cometabci.ResponseProcessProposal{Status: cometabci.ResponseProcessProposal_REJECT}, err
}
Expand Down Expand Up @@ -173,8 +181,8 @@ func (h *ProposalHandler) generateExchangeRateVotes(
continue
}

var voteExt OracleVoteExtension
if err := json.Unmarshal(vote.VoteExtension, &voteExt); err != nil {
var voteExt oracletypes.OracleVoteExtension
if err := voteExt.Unmarshal(vote.VoteExtension); err != nil {
h.logger.Error(
"failed to decode vote extension",
"err", err,
Expand Down
25 changes: 14 additions & 11 deletions x/oracle/abci/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package abci_test

import (
"bytes"
"encoding/json"
"sort"

"cosmossdk.io/core/header"
Expand Down Expand Up @@ -106,8 +105,8 @@ func (s *IntegrationTestSuite) TestPrepareProposalHandler() {
s.Require().NoError(err)
s.Require().NotNil(resp)

var injectedVoteExtTx abci.AggregateExchangeRateVotes
err = json.Unmarshal(resp.Txs[0], &injectedVoteExtTx)
var injectedVoteExtTx oracletypes.InjectedVoteExtensionTx
err = injectedVoteExtTx.Unmarshal(resp.Txs[0])
s.Require().NoError(err)

sort.Slice(valKeys[:], func(i, j int) bool {
Expand Down Expand Up @@ -168,21 +167,25 @@ func (s *IntegrationTestSuite) TestProcessProposalHandler() {
Voter: valKeys[1].ValAddress.String(),
},
}
injectedVoteExtTx := abci.AggregateExchangeRateVotes{
localCommitInfoBz, err := localCommitInfo.Marshal()
s.Require().NoError(err)
injectedVoteExtTx := oracletypes.InjectedVoteExtensionTx{
ExchangeRateVotes: exchangeRateVotes,
ExtendedCommitInfo: localCommitInfo,
ExtendedCommitInfo: localCommitInfoBz,
}
bz, err := json.Marshal(injectedVoteExtTx)
bz, err := injectedVoteExtTx.Marshal()
s.Require().NoError(err)
var txs [][]byte
txs = append(txs, bz)

// create tx with conflicting local commit info
injectedVoteExtTxConflicting := abci.AggregateExchangeRateVotes{
localCommitInfoConflictingBz, err := localCommitInfoConflicting.Marshal()
s.Require().NoError(err)
injectedVoteExtTxConflicting := oracletypes.InjectedVoteExtensionTx{
ExchangeRateVotes: exchangeRateVotes,
ExtendedCommitInfo: localCommitInfoConflicting,
ExtendedCommitInfo: localCommitInfoConflictingBz,
}
bz, err = json.Marshal(injectedVoteExtTxConflicting)
bz, err = injectedVoteExtTxConflicting.Marshal()
s.Require().NoError(err)
var txsConflicting [][]byte
txsConflicting = append(txsConflicting, bz)
Expand Down Expand Up @@ -302,11 +305,11 @@ func buildLocalCommitInfo(
valKeys [2]integration.TestValidatorKey,
chainID string,
) (cometabci.ExtendedCommitInfo, error) {
voteExt := abci.OracleVoteExtension{
voteExt := oracletypes.OracleVoteExtension{
ExchangeRates: exchangeRates,
Height: ctx.BlockHeight(),
}
bz, err := json.Marshal(voteExt)
bz, err := voteExt.Marshal()
if err != nil {
return cometabci.ExtendedCommitInfo{}, err
}
Expand Down
28 changes: 16 additions & 12 deletions x/oracle/abci/voteextension.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package abci

import (
"encoding/json"
"fmt"

"cosmossdk.io/log"
Expand All @@ -13,12 +12,6 @@ import (
"github.com/ojo-network/price-feeder/oracle"
)

// OracleVoteExtension defines the canonical vote extension structure.
type OracleVoteExtension struct {
Height int64
ExchangeRates sdk.DecCoins
}

type VoteExtensionHandler struct {
logger log.Logger
oracleKeeper keeper.Keeper
Expand Down Expand Up @@ -64,6 +57,7 @@ func (h *VoteExtensionHandler) ExtendVoteHandler() sdk.ExtendVoteHandler {
h.logger.Error(err.Error())
return &cometabci.ResponseExtendVote{VoteExtension: []byte{}}, err
}

prices := h.oracleKeeper.PriceFeeder.Oracle.GetPrices()
exchangeRatesStr := oracle.GenerateExchangeRatesString(prices)

Expand All @@ -80,19 +74,19 @@ func (h *VoteExtensionHandler) ExtendVoteHandler() sdk.ExtendVoteHandler {

// Filter out rates which aren't included in the AcceptList.
acceptList := h.oracleKeeper.AcceptList(ctx)
filteredDecCoins := sdk.DecCoins{}
filteredDecCoins := []sdk.DecCoin{}
for _, decCoin := range exchangeRates {
if acceptList.Contains(decCoin.Denom) {
filteredDecCoins = append(filteredDecCoins, decCoin)
}
}

voteExt := OracleVoteExtension{
voteExt := types.OracleVoteExtension{
Height: req.Height,
ExchangeRates: filteredDecCoins,
}

bz, err := json.Marshal(voteExt)
bz, err := voteExt.Marshal()
if err != nil {
err := fmt.Errorf("failed to marshal vote extension: %w", err)
h.logger.Error(
Expand All @@ -101,6 +95,7 @@ func (h *VoteExtensionHandler) ExtendVoteHandler() sdk.ExtendVoteHandler {
)
return &cometabci.ResponseExtendVote{VoteExtension: []byte{}}, err
}

h.logger.Info(
"created vote extension",
"height", req.Height,
Expand All @@ -123,8 +118,17 @@ func (h *VoteExtensionHandler) VerifyVoteExtensionHandler() sdk.VerifyVoteExtens
return nil, err
}

var voteExt OracleVoteExtension
err := json.Unmarshal(req.VoteExtension, &voteExt)
if len(req.VoteExtension) == 0 {
h.logger.Info(
"verify vote extension handler received empty vote extension",
"height", req.Height,
)

return &cometabci.ResponseVerifyVoteExtension{Status: cometabci.ResponseVerifyVoteExtension_ACCEPT}, nil
}

var voteExt types.OracleVoteExtension
err := voteExt.Unmarshal(req.VoteExtension)
if err != nil {
err := fmt.Errorf("verify vote extension handler failed to unmarshal vote extension: %w", err)
h.logger.Error(
Expand Down
19 changes: 9 additions & 10 deletions x/oracle/abci/voteextension_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package abci_test

import (
"encoding/json"
"fmt"

"cosmossdk.io/log"
Expand All @@ -10,6 +9,7 @@ import (
"github.com/ojo-network/ojo/pricefeeder"
"github.com/ojo-network/ojo/x/oracle/abci"
"github.com/ojo-network/ojo/x/oracle/keeper"
"github.com/ojo-network/ojo/x/oracle/types"
)

func (s *IntegrationTestSuite) TestExtendVoteHandler() {
Expand Down Expand Up @@ -41,8 +41,7 @@ func (s *IntegrationTestSuite) TestExtendVoteHandler() {
extendVoteRequest: &cometabci.RequestExtendVote{
Height: ctx.BlockHeight(),
},
expErr: true,
expErrMsg: "price feeder oracle not set",
expErr: true,
},
{
name: "vote extension handled successfully",
Expand Down Expand Up @@ -71,10 +70,9 @@ func (s *IntegrationTestSuite) TestExtendVoteHandler() {
} else {
s.Require().NoError(err)
s.Require().NotNil(resp)
s.Require().Greater(len(resp.VoteExtension), 0)

var voteExt abci.OracleVoteExtension
err = json.Unmarshal(resp.VoteExtension, &voteExt)
var voteExt types.OracleVoteExtension
err = voteExt.Unmarshal(resp.VoteExtension)
s.Require().NoError(err)
s.Require().Equal(ctx.BlockHeight(), voteExt.Height)
}
Expand All @@ -86,9 +84,10 @@ func (s *IntegrationTestSuite) TestVerifyVoteExtensionHandler() {
app, ctx := s.app, s.ctx
pf := MockPriceFeeder()

voteExtension, err := json.Marshal(&cometabci.RequestExtendVote{
voteExtension := cometabci.RequestExtendVote{
Height: ctx.BlockHeight(),
})
}
voteExtensionBz, err := voteExtension.Marshal()
s.Require().NoError(err)

testCases := []struct {
Expand All @@ -115,7 +114,7 @@ func (s *IntegrationTestSuite) TestVerifyVoteExtensionHandler() {
priceFeeder: pf,
verifyVoteRequest: &cometabci.RequestVerifyVoteExtension{
Height: ctx.BlockHeight() + 1,
VoteExtension: voteExtension,
VoteExtension: voteExtensionBz,
},
expErr: true,
expErrMsg: fmt.Sprintf("verify vote extension handler received vote extension height that doesn't"+
Expand All @@ -131,7 +130,7 @@ func (s *IntegrationTestSuite) TestVerifyVoteExtensionHandler() {
priceFeeder: pf,
verifyVoteRequest: &cometabci.RequestVerifyVoteExtension{
Height: ctx.BlockHeight(),
VoteExtension: voteExtension,
VoteExtension: voteExtensionBz,
},
expErr: false,
},
Expand Down
Loading
Loading