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

[SKI-1] Remove smoothed prices #1215

Merged
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
1 change: 0 additions & 1 deletion protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,6 @@ func New(
appCodec,
keys[pricesmoduletypes.StoreKey],
indexPriceCache,
pricesmoduletypes.NewMarketToSmoothedPrices(pricesmoduletypes.SmoothedPriceTrackingBlockHistoryLength),
timeProvider,
app.IndexerEventManager,
// set the governance and delaymsg module accounts as the authority for conducting upgrades
Comment on lines 876 to 881
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [738-738]

The gRPC server is initialized without specifying credentials, which could potentially expose the server to unencrypted connections. It's recommended to secure the gRPC server by using SSL/TLS credentials. You can create credentials using credentials.NewServerTLSFromFile("cert.pem", "cert.key") and pass them to the grpc.NewServer function using the grpc.Creds() option.

- app.Server = daemonserver.NewServer(logger, grpc.NewServer(), &daemontypes.FileHandlerImpl{}, daemonFlags.Shared.SocketAddress)
+ creds, err := credentials.NewServerTLSFromFile("cert.pem", "cert.key")
+ if err != nil {
+     panic(err)
+ }
+ app.Server = daemonserver.NewServer(logger, grpc.NewServer(grpc.Creds(creds)), &daemontypes.FileHandlerImpl{}, daemonFlags.Shared.SocketAddress)

Expand Down
4 changes: 2 additions & 2 deletions protocol/app/prepare/prepare_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func TestPrepareProposalHandler(t *testing.T) {
mockClobKeeper.On("GetOperations", mock.Anything, mock.Anything).
Return(tc.clobResp)

ctx, _, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, _, _, _, _ := keepertest.PricesKeepers(t)

handler := prepare.PrepareProposalHandler(
mockTxConfig,
Expand Down Expand Up @@ -440,7 +440,7 @@ func TestPrepareProposalHandler_OtherTxs(t *testing.T) {
mockBridgeKeeper.On("GetAcknowledgeBridges", mock.Anything, mock.Anything).
Return(constants.MsgAcknowledgeBridges_Ids0_1_Height0)

ctx, _, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, _, _, _, _ := keepertest.PricesKeepers(t)

handler := prepare.PrepareProposalHandler(
encodingCfg.TxConfig,
Expand Down
5 changes: 0 additions & 5 deletions protocol/app/process/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ type ProcessPricesKeeper interface {
marketPriceUpdates *pricestypes.MsgUpdateMarketPrices,
performNonDeterministicValidation bool,
) error

UpdateSmoothedPrices(
ctx sdk.Context,
linearInterpolateFunc func(v0 uint64, v1 uint64, ppm uint32) (uint64, error),
) error
}

// ProcessClobKeeper defines the expected clob keeper used for `ProcessProposal`.
Expand Down
2 changes: 1 addition & 1 deletion protocol/app/process/full_node_process_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestFullNodeProcessProposalHandler(t *testing.T) {
// Setup.
_, bridgeKeeper, _, _, _, _, _ := keepertest.BridgeKeepers(t)

ctx, pricesKeeper, _, indexPriceCache, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, indexPriceCache, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
keepertest.CreateTestMarkets(t, ctx, pricesKeeper)
indexPriceCache.UpdatePrices(constants.AtTimeTSingleExchangePriceUpdate)
Expand Down
6 changes: 3 additions & 3 deletions protocol/app/process/market_prices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestDecodeUpdateMarketPricesTx(t *testing.T) {

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
ctx, k, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, k, _, _, _ := keepertest.PricesKeepers(t)
pricesTxDecoder := process.NewDefaultUpdateMarketPriceTxDecoder(k, encodingCfg.TxConfig.TxDecoder())
umpt, err := pricesTxDecoder.DecodeUpdateMarketPricesTx(ctx, [][]byte{tc.txBytes})
if tc.expectedErr != nil {
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestUpdateMarketPricesTx_Validate(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Setup.
ctx, k, _, indexPriceCache, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, k, _, indexPriceCache, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
keepertest.CreateTestMarkets(t, ctx, k)
indexPriceCache.UpdatePrices(tc.indexPrices)
Expand Down Expand Up @@ -162,7 +162,7 @@ func TestUpdateMarketPricesTx_GetMsg(t *testing.T) {
t.Run(name, func(t *testing.T) {
var msg sdk.Msg
if tc.txBytes != nil {
ctx, k, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, k, _, _, _ := keepertest.PricesKeepers(t)

// Decode.
pricesTxDecoder := process.NewDefaultUpdateMarketPriceTxDecoder(k, constants.TestEncodingCfg.TxConfig.TxDecoder())
Expand Down
9 changes: 0 additions & 9 deletions protocol/app/process/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package process
import (
"time"

"github.com/dydxprotocol/v4-chain/protocol/lib"
error_lib "github.com/dydxprotocol/v4-chain/protocol/lib/error"
"github.com/dydxprotocol/v4-chain/protocol/lib/log"

Expand Down Expand Up @@ -65,14 +64,6 @@ func ProcessProposalHandler(
log.Module, ModuleName,
)

// Perform the update of smoothed prices here to ensure that smoothed prices are updated even if a block is later
// rejected by consensus. We want smoothed prices to be updated on fixed cadence, and we are piggybacking on
// consensus round to do so.
if err := pricesKeeper.UpdateSmoothedPrices(ctx, lib.Uint64LinearInterpolate); err != nil {
recordErrorMetricsWithLabel(metrics.UpdateSmoothedPrices)
error_lib.LogErrorWithOptionalContext(ctx, "UpdateSmoothedPrices failed", err)
}

txs, err := DecodeProcessProposalTxs(ctx, txConfig.TxDecoder(), req, bridgeKeeper, pricesTxDecoder)
if err != nil {
error_lib.LogErrorWithOptionalContext(ctx, "DecodeProcessProposalTxs failed", err)
Expand Down
7 changes: 1 addition & 6 deletions protocol/app/process/process_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func TestProcessProposalHandler_Error(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Setup.
ctx, pricesKeeper, _, indexPriceCache, marketToSmoothedPrices, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, indexPriceCache, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
keepertest.CreateTestMarkets(t, ctx, pricesKeeper)
indexPriceCache.UpdatePrices(constants.AtTimeTSingleExchangePriceUpdate)
Expand Down Expand Up @@ -295,11 +295,6 @@ func TestProcessProposalHandler_Error(t *testing.T) {

// Validate.
require.Equal(t, tc.expectedResponse, *resp)
require.Equal(
t,
marketToSmoothedPrices.GetSmoothedPricesForTest(),
constants.AtTimeTSingleExchangeSmoothedPrices,
)
})
}
}
8 changes: 4 additions & 4 deletions protocol/app/process/transactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestDecodeProcessProposalTxs_Error(t *testing.T) {
t.Run(name, func(t *testing.T) {
// Setup.
_, bridgeKeeper, _, _, _, _, _ := keepertest.BridgeKeepers(t)
ctx, pricesKeeper, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, _, _ := keepertest.PricesKeepers(t)

// Run.
_, err := process.DecodeProcessProposalTxs(
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestDecodeProcessProposalTxs_Valid(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Setup.
ctx, pricesKeeper, _, _, _, _ := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, _, _ := keepertest.PricesKeepers(t)
_, bridgeKeeper, _, _, _, _, _ := keepertest.BridgeKeepers(t)

// Run.
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestProcessProposalTxs_Validate_Error(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Setup.
ctx, pricesKeeper, _, indexPriceCache, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, indexPriceCache, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
keepertest.CreateTestMarkets(t, ctx, pricesKeeper)
indexPriceCache.UpdatePrices(constants.AtTimeTSingleExchangePriceUpdate)
Expand Down Expand Up @@ -425,7 +425,7 @@ func TestProcessProposalTxs_Validate_Valid(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Setup.
ctx, pricesKeeper, _, indexPriceCache, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, pricesKeeper, _, indexPriceCache, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
keepertest.CreateTestMarkets(t, ctx, pricesKeeper)
indexPriceCache.UpdatePrices(constants.AtTimeTSingleExchangePriceUpdate)
Expand Down
36 changes: 16 additions & 20 deletions protocol/lib/metrics/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,26 +255,22 @@ const (
BlockTimeMs = "block_time_ms"

// Prices.
CreateOracleMarket = "create_oracle_market"
CurrentMarketPrices = "current_market_prices"
GetValidMarketPriceUpdates = "get_valid_market_price_updates"
IndexPriceDoesNotExist = "index_price_does_not_exist"
IndexPriceIsZero = "index_price_is_zero"
IndexPriceNotAccurate = "index_price_not_accurate"
IndexPriceNotAvailForAccuracyCheck = "index_price_not_available_for_accuracy_check"
LastPriceUpdateForMarketBlock = "last_price_update_for_market_block"
MissingPriceUpdates = "missing_price_updates"
NumMarketPricesToUpdate = "num_market_prices_to_update"
PriceChangeRate = "price_change_rate"
ProposedPriceChangesPriceUpdateDecision = "proposed_price_changes_price_update_decision"
ProposedPriceCrossesOraclePrice = "proposed_price_crosses_oracle_price"
ProposedPriceDoesNotMeetMinPriceChange = "proposed_price_does_not_meet_min_price_change"
RecentSmoothedPriceDoesNotMeetMinPriceChange = "recent_smoothed_price_doesnt_meet_min_price_change"
RecentSmoothedPriceCrossesOraclePrice = "recent_smoothed_price_crosses_old_price"
StatefulPriceUpdateValidation = "stateful_price_update_validation"
UpdateMarketParam = "update_market_param"
UpdateMarketPrices = "update_market_prices"
UpdateSmoothedPrices = "update_smoothed_prices"
CreateOracleMarket = "create_oracle_market"
CurrentMarketPrices = "current_market_prices"
GetValidMarketPriceUpdates = "get_valid_market_price_updates"
IndexPriceDoesNotExist = "index_price_does_not_exist"
IndexPriceIsZero = "index_price_is_zero"
IndexPriceNotAccurate = "index_price_not_accurate"
IndexPriceNotAvailForAccuracyCheck = "index_price_not_available_for_accuracy_check"
LastPriceUpdateForMarketBlock = "last_price_update_for_market_block"
MissingPriceUpdates = "missing_price_updates"
NumMarketPricesToUpdate = "num_market_prices_to_update"
PriceChangeRate = "price_change_rate"
ProposedPriceChangesPriceUpdateDecision = "proposed_price_changes_price_update_decision"
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The search results confirm that the old constant name ProposedPriceDoesNotMeetMinPriceChange is still being used in the codebase, indicating that not all references to it have been updated as expected. This supports the initial review comment.

Analysis chain

The renaming of ProposedPriceDoesNotMeetMinPriceChange to ProposedPriceChangesPriceUpdateDecision is noted. Ensure all references to the old constant name have been updated throughout the codebase.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the old constant name to ensure it has been updated everywhere.
rg --type go 'ProposedPriceDoesNotMeetMinPriceChange'

Length of output: 510

ProposedPriceDoesNotMeetMinPriceChange = "proposed_price_does_not_meet_min_price_change"
StatefulPriceUpdateValidation = "stateful_price_update_validation"
UpdateMarketParam = "update_market_param"
UpdateMarketPrices = "update_market_prices"

// Sending.
Account = "account"
Expand Down
18 changes: 0 additions & 18 deletions protocol/mocks/PricesKeeper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion protocol/testutil/keeper/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func AssetsKeepers(
transientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
pricesKeeper, _, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
accountKeeper, _ = createAccountKeeper(stateStore, db, cdc, registry)
bankKeeper, _ = createBankKeeper(stateStore, db, cdc, accountKeeper)
keeper, storeKey = createAssetsKeeper(stateStore, db, cdc, pricesKeeper, transientStoreKey, msgSenderEnabled)
Expand Down
2 changes: 1 addition & 1 deletion protocol/testutil/keeper/clob.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
indexerEventsTransientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
ks.PricesKeeper, _, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, indexerEventsTransientStoreKey)
ks.PricesKeeper, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, indexerEventsTransientStoreKey)
// Mock time provider response for market creation.
mockTimeProvider.On("Now").Return(constants.TimeT)
epochsKeeper, _ := createEpochsKeeper(stateStore, db, cdc)
Expand Down
2 changes: 1 addition & 1 deletion protocol/testutil/keeper/perpetuals.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func PerpetualsKeepersWithClobHelpers(
transientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
pc.PricesKeeper, _, pc.IndexPriceCache, _, pc.MockTimeProvider = createPricesKeeper(
pc.PricesKeeper, _, pc.IndexPriceCache, pc.MockTimeProvider = createPricesKeeper(
stateStore,
db,
cdc,
Expand Down
11 changes: 3 additions & 8 deletions protocol/testutil/keeper/prices.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ func PricesKeepers(
keeper *keeper.Keeper,
storeKey storetypes.StoreKey,
indexPriceCache *pricefeedserver_types.MarketToExchangePrices,
marketToSmoothedPrices types.MarketToSmoothedPrices,
mockTimeProvider *mocks.TimeProvider,
) {
ctx = initKeepers(t, func(
Expand All @@ -42,13 +41,13 @@ func PricesKeepers(
transientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
keeper, storeKey, indexPriceCache, marketToSmoothedPrices, mockTimeProvider =
keeper, storeKey, indexPriceCache, mockTimeProvider =
createPricesKeeper(stateStore, db, cdc, transientStoreKey)

return []GenesisInitializer{keeper}
})

return ctx, keeper, storeKey, indexPriceCache, marketToSmoothedPrices, mockTimeProvider
return ctx, keeper, storeKey, indexPriceCache, mockTimeProvider
}

func createPricesKeeper(
Expand All @@ -60,7 +59,6 @@ func createPricesKeeper(
*keeper.Keeper,
storetypes.StoreKey,
*pricefeedserver_types.MarketToExchangePrices,
types.MarketToSmoothedPrices,
*mocks.TimeProvider,
) {
storeKey := storetypes.NewKVStoreKey(types.StoreKey)
Expand All @@ -69,8 +67,6 @@ func createPricesKeeper(

indexPriceCache := pricefeedserver_types.NewMarketToExchangePrices(pricefeed_types.MaxPriceAge)

marketToSmoothedPrices := types.NewMarketToSmoothedPrices(types.SmoothedPriceTrackingBlockHistoryLength)

mockTimeProvider := &mocks.TimeProvider{}

mockMsgSender := &mocks.IndexerMessageSender{}
Expand All @@ -84,7 +80,6 @@ func createPricesKeeper(
cdc,
storeKey,
indexPriceCache,
marketToSmoothedPrices,
mockTimeProvider,
mockIndexerEventsManager,
[]string{
Expand All @@ -93,7 +88,7 @@ func createPricesKeeper(
},
)

return k, storeKey, indexPriceCache, marketToSmoothedPrices, mockTimeProvider
return k, storeKey, indexPriceCache, mockTimeProvider
}

// CreateTestMarkets creates a standard set of test markets for testing.
Expand Down
2 changes: 1 addition & 1 deletion protocol/testutil/keeper/rewards.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func RewardsKeepers(
transientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
pricesKeeper, _, _, _, _ = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
pricesKeeper, _, _, _ = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
// Mock time provider response for market creation.
epochsKeeper, _ := createEpochsKeeper(stateStore, db, cdc)
assetsKeeper, _ = createAssetsKeeper(
Expand Down
2 changes: 1 addition & 1 deletion protocol/testutil/keeper/sending.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func SendingKeepersWithSubaccountsKeeper(t testing.TB, saKeeper types.Subaccount
// Define necessary keepers here for unit tests
epochsKeeper, _ := createEpochsKeeper(stateStore, db, cdc)
blockTimeKeeper, _ := createBlockTimeKeeper(stateStore, db, cdc)
ks.PricesKeeper, _, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
ks.PricesKeeper, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
ks.PerpetualsKeeper, _ = createPerpetualsKeeper(
stateStore,
db,
Expand Down
2 changes: 1 addition & 1 deletion protocol/testutil/keeper/subaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func SubaccountsKeepers(
transientStoreKey storetypes.StoreKey,
) []GenesisInitializer {
// Define necessary keepers here for unit tests
pricesKeeper, _, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
pricesKeeper, _, _, mockTimeProvider = createPricesKeeper(stateStore, db, cdc, transientStoreKey)
epochsKeeper, _ := createEpochsKeeper(stateStore, db, cdc)
perpetualsKeeper, _ = createPerpetualsKeeper(stateStore, db, cdc, pricesKeeper, epochsKeeper, transientStoreKey)
assetsKeeper, _ = createAssetsKeeper(stateStore, db, cdc, pricesKeeper, transientStoreKey, msgSenderEnabled)
Expand Down
8 changes: 4 additions & 4 deletions protocol/x/prices/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestExportGenesis(t *testing.T) {
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
ctx, k, _, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, k, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)

prices.InitGenesis(ctx, *k, *tc.genesisState)
Expand All @@ -50,7 +50,7 @@ func TestExportGenesis(t *testing.T) {
}

func TestExportGenesis_WithMutation(t *testing.T) {
ctx, k, _, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, k, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)
prices.InitGenesis(ctx, *k, *types.DefaultGenesis())

Expand Down Expand Up @@ -94,7 +94,7 @@ func invalidGenesis() types.GenesisState {
}

func TestInitGenesis_Panics(t *testing.T) {
ctx, k, _, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, k, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)

// Verify InitGenesis panics when given an invalid genesis state.
Expand All @@ -104,7 +104,7 @@ func TestInitGenesis_Panics(t *testing.T) {
}

func TestInitGenesisEmitsMarketUpdates(t *testing.T) {
ctx, k, _, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
ctx, k, _, _, mockTimeProvider := keepertest.PricesKeepers(t)
mockTimeProvider.On("Now").Return(constants.TimeT)

prices.InitGenesis(ctx, *k, constants.Prices_DefaultGenesisState)
Expand Down
Loading
Loading