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

feat: make PML compatible with OE by staging in-memory CLOB side effects #2447

Merged
merged 4 commits into from
Oct 3, 2024
+1,024 −413
Merged

feat: make PML compatible with OE by staging in-memory CLOB side effects #2447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a9482b
Stage clob pair creation
teddyding Oct 3, 2024
46c33fe
comments
teddyding Oct 3, 2024
0465101
Add commentt
teddyding Oct 3, 2024
48b44d3
Add comment on Precommit
teddyding Oct 3, 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
516 changes: 259 additions & 257 deletions indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts
View file
Open in desktop

Large diffs are not rendered by default.

66 changes: 66 additions & 0 deletions indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/finalize_block.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { ClobPair, ClobPairSDKType } from "./clob_pair";
import * as _m0 from "protobufjs/minimal";
import { DeepPartial } from "../../helpers";
/**
* ClobStagedFinalizeBlockEvent defines a CLOB event staged during
* FinalizeBlock.
*/

export interface ClobStagedFinalizeBlockEvent {
/** create_clob_pair indicates a new CLOB pair creation. */
createClobPair?: ClobPair;
}
/**
* ClobStagedFinalizeBlockEvent defines a CLOB event staged during
* FinalizeBlock.
*/

export interface ClobStagedFinalizeBlockEventSDKType {
/** create_clob_pair indicates a new CLOB pair creation. */
create_clob_pair?: ClobPairSDKType;
}

function createBaseClobStagedFinalizeBlockEvent(): ClobStagedFinalizeBlockEvent {
return {
createClobPair: undefined
};
}

export const ClobStagedFinalizeBlockEvent = {
encode(message: ClobStagedFinalizeBlockEvent, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
if (message.createClobPair !== undefined) {
ClobPair.encode(message.createClobPair, writer.uint32(10).fork()).ldelim();
}

return writer;
},

decode(input: _m0.Reader | Uint8Array, length?: number): ClobStagedFinalizeBlockEvent {
const reader = input instanceof _m0.Reader ? input : new _m0.Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = createBaseClobStagedFinalizeBlockEvent();

while (reader.pos < end) {
const tag = reader.uint32();

switch (tag >>> 3) {
case 1:
message.createClobPair = ClobPair.decode(reader, reader.uint32());
break;

default:
reader.skipType(tag & 7);
break;
}
}

return message;
},

fromPartial(object: DeepPartial<ClobStagedFinalizeBlockEvent>): ClobStagedFinalizeBlockEvent {
const message = createBaseClobStagedFinalizeBlockEvent();
message.createClobPair = object.createClobPair !== undefined && object.createClobPair !== null ? ClobPair.fromPartial(object.createClobPair) : undefined;
return message;
}

};
4 changes: 2 additions & 2 deletions indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import * as _128 from "./gogo";
export const gogoproto = { ..._128
import * as _129 from "./gogo";
export const gogoproto = { ..._129
};
22 changes: 11 additions & 11 deletions indexer/packages/v4-protos/src/codegen/google/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import * as _129 from "./api/annotations";
import * as _130 from "./api/http";
import * as _131 from "./protobuf/descriptor";
import * as _132 from "./protobuf/duration";
import * as _133 from "./protobuf/timestamp";
import * as _134 from "./protobuf/any";
import * as _130 from "./api/annotations";
import * as _131 from "./api/http";
import * as _132 from "./protobuf/descriptor";
import * as _133 from "./protobuf/duration";
import * as _134 from "./protobuf/timestamp";
import * as _135 from "./protobuf/any";
export namespace google {
export const api = { ..._129,
..._130
export const api = { ..._130,
..._131
};
export const protobuf = { ..._131,
..._132,
export const protobuf = { ..._132,
..._133,
..._134
..._134,
..._135
};
}
16 changes: 16 additions & 0 deletions proto/dydxprotocol/clob/finalize_block.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
syntax = "proto3";
package dydxprotocol.clob;

import "dydxprotocol/clob/clob_pair.proto";

option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/clob/types";

// ClobStagedFinalizeBlockEvent defines a CLOB event staged during
// FinalizeBlock.
message ClobStagedFinalizeBlockEvent {
// event is the staged event.
oneof event {
// create_clob_pair indicates a new CLOB pair creation.
ClobPair create_clob_pair = 1;
}
}
17 changes: 15 additions & 2 deletions protocol/mocks/MemClob.go
View file
Open in desktop

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

4 changes: 2 additions & 2 deletions protocol/testutil/keeper/clob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func CreateTestClobPairs(
clobPairs []types.ClobPair,
) {
for _, clobPair := range clobPairs {
_, err := clobKeeper.CreatePerpetualClobPair(
_, err := clobKeeper.CreatePerpetualClobPairAndMemStructs(
ctx,
clobPair.Id,
clobPair.MustGetPerpetualId(),
Expand Down Expand Up @@ -352,7 +352,7 @@ func CreateNClobPair(
),
).Return()

_, err := keeper.CreatePerpetualClobPair(
_, err := keeper.CreatePerpetualClobPairAndMemStructs(
ctx,
items[i].Id,
clobtest.MustPerpetualId(items[i]),
Expand Down
6 changes: 6 additions & 0 deletions protocol/x/clob/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ func Precommit(
ctx sdk.Context,
keeper keeper.Keeper,
) {
// Process all staged finalize block events, and apply necessary side effects
// (e.g. MemClob orderbook creation) that could not be done during FinalizeBlock.
// Note: this must be done in `Precommit` which is prior to `PrepareCheckState`, when
// MemClob could access the new orderbooks.
keeper.ProcessStagedFinalizeBlockEvents(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment as to why this needs to be here?


if streamingManager := keeper.GetFullNodeStreamingManager(); !streamingManager.Enabled() {
return
}
Expand Down
6 changes: 3 additions & 3 deletions protocol/x/clob/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func TestEndBlocker_Success(t *testing.T) {
),
),
).Once().Return()
_, err = ks.ClobKeeper.CreatePerpetualClobPair(
_, err = ks.ClobKeeper.CreatePerpetualClobPairAndMemStructs(
ctx,
constants.ClobPair_Btc.Id,
clobtest.MustPerpetualId(constants.ClobPair_Btc),
Expand Down Expand Up @@ -563,7 +563,7 @@ func TestEndBlocker_Success(t *testing.T) {
),
),
).Once().Return()
_, err = ks.ClobKeeper.CreatePerpetualClobPair(
_, err = ks.ClobKeeper.CreatePerpetualClobPairAndMemStructs(
ctx,
constants.ClobPair_Eth.Id,
clobtest.MustPerpetualId(constants.ClobPair_Eth),
Expand Down Expand Up @@ -1170,7 +1170,7 @@ func TestPrepareCheckState(t *testing.T) {

// Create all CLOBs.
for _, clobPair := range tc.clobs {
_, err = ks.ClobKeeper.CreatePerpetualClobPair(
_, err = ks.ClobKeeper.CreatePerpetualClobPairAndMemStructs(
ctx,
clobPair.Id,
clobtest.MustPerpetualId(clobPair),
Expand Down
2 changes: 1 addition & 1 deletion protocol/x/clob/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func InitGenesis(ctx sdk.Context, k *keeper.Keeper, genState types.GenesisState)
if err != nil {
panic(errorsmod.Wrap(types.ErrInvalidClobPairParameter, err.Error()))
}
_, err = k.CreatePerpetualClobPair(
_, err = k.CreatePerpetualClobPairAndMemStructs(
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing Implementation of CreateOrderbook Method

The CreatePerpetualClobPairAndMemStructs function in protocol/x/clob/genesis.go calls the CreateOrderbook method of MemClob. However, the CreateOrderbook method was not found in the codebase, indicating it may be missing or not implemented.

  • File: protocol/x/clob/keeper/clob_pair.go
  • Method Call: k.MemClob.CreateOrderbook(clobPair)

Please ensure that the CreateOrderbook method is properly implemented within the MemClob struct to handle in-memory order book creation.

🔗 Analysis chain

LGTM. Please verify new method implementation and confirm issue resolution.

The change from CreatePerpetualClobPair to CreatePerpetualClobPairAndMemStructs aligns with the PR objectives of making PML compatible with OE by staging ClobPair creation. This modification appears to address the issues mentioned in the PR description.

To ensure the completeness of this change:

  1. Please confirm that the CreatePerpetualClobPairAndMemStructs method in the keeper package properly implements the staging of ClobPair creation.
  2. Verify that this change effectively addresses the following issues mentioned in the PR objectives:
    • Discarding in-memory changes if the user transaction is reverted.
    • Handling the case when the block is processed optimistically and later discarded.

Can you provide more details on how these specific issues are resolved by this change?

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the implementation of CreatePerpetualClobPairAndMemStructs
ast-grep --lang go --pattern $'func (k *Keeper) CreatePerpetualClobPairAndMemStructs(
  $$$
) ($_, error) {
  $$$
}'

Length of output: 117

Script:

#!/bin/bash
# Search for the function CreatePerpetualClobPairAndMemStructs across all Go files
rg --type go 'CreatePerpetualClobPairAndMemStructs'

Length of output: 5627

Script:

#!/bin/bash
# Extract the implementation of CreatePerpetualClobPairAndMemStructs function
rg --type go -A 50 'func \(k Keeper\) CreatePerpetualClobPairAndMemStructs' protocol/x/clob/keeper/clob_pair.go

Length of output: 1584

Script:

#!/bin/bash
# Extract the implementation of createPerpetualClobPair function
rg --type go -A 50 'func \(k Keeper\) createPerpetualClobPair' protocol/x/clob/keeper/clob_pair.go

# Extract the implementation of CreateOrderbook method in MemClob
rg --type go -A 50 'func \(m \*MemClob\) CreateOrderbook' protocol/x/clob/keeper/memclob/memclob.go

# Extract the implementation of SetClobPairIdForPerpetual function
rg --type go -A 50 'func \(k Keeper\) SetClobPairIdForPerpetual' protocol/x/clob/keeper/clob_pair.go

Length of output: 3421

Script:

#!/bin/bash
# Search for the CreateOrderbook method across all Go files
rg --type go 'func \(m \*MemClob\) CreateOrderbook'

Length of output: 53

ctx,
elem.Id,
perpetualId,
Expand Down
Loading
Loading