Skip to content

Commit

Permalink
fix: Switch from stdlib json encoding to proto encoding in vote exten…
Browse files Browse the repository at this point in the history
…sion handler (#516)

* fix: Create vote extension with no exchange rates if price feeder oracle is not set

* debug log

* fix debug statement

* handle empty ve

* fix: Switch from stdlib jsosn encoding to proto encoding in vote extension handler

* lint

* commit info error

* move txs len check to correct place

* err in endblocker
  • Loading branch information
rbajollari authored Sep 23, 2024
1 parent 960e5e1 commit 65e8943
Show file tree
Hide file tree
Showing 9 changed files with 685 additions and 54 deletions.
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()
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

0 comments on commit 65e8943

Please sign in to comment.