[TRA-572] Add upsert vault event. #2250
Conversation
WalkthroughThe changes involve a significant restructuring of the Changes
Possibly related PRs
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (3)
Files skipped from review due to trivial changes (2)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
protocol/indexer/events/events.pb.gois excluded by!**/*.pb.goprotocol/indexer/protocol/v1/types/vault.pb.gois excluded by!**/*.pb.go
Files selected for processing (6)
- proto/dydxprotocol/indexer/events/events.proto (2 hunks)
- proto/dydxprotocol/indexer/protocol/v1/vault.proto (1 hunks)
- protocol/indexer/events/upsert_vault.go (1 hunks)
- protocol/indexer/events/upsert_vault_test.go (1 hunks)
- protocol/indexer/protocol/v1/v1_mappers.go (2 hunks)
- protocol/indexer/protocol/v1/v1_mappers_test.go (2 hunks)
Additional context used
buf
proto/dydxprotocol/indexer/protocol/v1/vault.proto
2-2: Files with package "dydxprotocol.indexer.protocol.v1" must be within a directory "dydxprotocol/indexer/protocol/v1" relative to root but were in directory "proto/dydxprotocol/indexer/protocol/v1".
(PACKAGE_DIRECTORY_MATCH)
Additional comments not posted (6)
protocol/indexer/events/upsert_vault.go (1)
11-21: LGTM!The function implementation looks good:
- The function signature and return type are appropriate.
- The struct fields are correctly populated using the function parameters.
- The
clobPairIdandstatusconversions are correct, assuming the types and conversion functions are properly defined.The function name
NewUpsertVaultEventand the struct nameUpsertVaultEventV1suggest that this event represents both creation and update of a vault.Please verify if separate events for vault creation and update are needed, or if a single upsert event is sufficient. This may depend on how the indexer processes these events and if it needs to differentiate between creation and update.
You can use the following script to search for usage of vault creation and update events in the codebase:
protocol/indexer/events/upsert_vault_test.go (1)
14-26: LGTM!The test function is correctly testing the creation of a new
UpsertVaultEventwith the expected parameters. The test is using the correct assertion to compare the actual and expected objects. The test is not missing any important scenarios or edge cases.proto/dydxprotocol/indexer/protocol/v1/vault.proto (1)
12-27: LGTM!The
VaultStatusenum definition follows the protobuf style guide and the values are well-documented.protocol/indexer/protocol/v1/v1_mappers.go (1)
210-212: LGTM!The
VaultStatusToIndexerVaultStatusfunction is correctly implemented. It simply casts the inputvaulttypes.VaultStatusto the outputv1types.VaultStatus.The function name clearly describes its purpose, and the implementation is straightforward and easy to understand.
protocol/indexer/protocol/v1/v1_mappers_test.go (1)
478-506: LGTM!The test function
TestVaultStatusToVaultStatusis well-structured and provides comprehensive test coverage for theVaultStatusToIndexerVaultStatusconversion function. It iterates over all possible values ofVaultStatusand ensures that the conversion produces the expected results using appropriate assertions.proto/dydxprotocol/indexer/events/events.proto (1)
594-605: LGTM!The new
UpsertVaultEventV1message is well-structured and captures the necessary information for a vault upsert event.Observations:
- The field names are clear and descriptive.
- Using the
cosmos.AddressStringscalar for theaddressfield promotes standardization.- Referencing the
VaultStatusenum fromvault.protois a good practice for code reuse and consistency.No issues found.
| @@ -0,0 +1,27 @@ | |||
| syntax = "proto3"; | |||
| package dydxprotocol.indexer.protocol.v1; | |||
There was a problem hiding this comment.
Fix the package directory to match the package name.
The static analysis tool buf has flagged an issue with the package directory not matching the package name. To fix this, move the file to the directory dydxprotocol/indexer/protocol/v1 relative to the root of the repository.
Apply this fix to resolve the buf issue:
- Create the directory
dydxprotocol/indexer/protocol/v1relative to the root of the repository if it doesn't exist. - Move the file
proto/dydxprotocol/indexer/protocol/v1/vault.prototo the new directorydydxprotocol/indexer/protocol/v1/vault.proto. - Update any import statements in other files that reference this file to use the new path.
Tools
buf
2-2: Files with package "dydxprotocol.indexer.protocol.v1" must be within a directory "dydxprotocol/indexer/protocol/v1" relative to root but were in directory "proto/dydxprotocol/indexer/protocol/v1".
(PACKAGE_DIRECTORY_MATCH)
| @@ -473,3 +474,33 @@ func TestConvertToPerpetualMarketType(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestVaultStatusToVaultStatus(t *testing.T) { | |||
There was a problem hiding this comment.
nit
| func TestVaultStatusToVaultStatus(t *testing.T) { | |
| func TestVaultStatusToIndexerVaultStatus(t *testing.T) { |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/protocol/v1/vault.ts (2)
38-65: LGTM with a minor suggestion!The
vaultStatusFromJSONfunction is correctly implemented and handles both number and string representations of the status.However, the case clauses at lines 60-61 are redundant because of the default clause. Please consider removing them to simplify the code.
Apply this diff to remove the redundant case clauses:
- case -1: - case "UNRECOGNIZED": default: return VaultStatus.UNRECOGNIZED;Tools
Biome
[error] 60-61: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 61-62: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
66-87: LGTM with a minor suggestion!The
vaultStatusToJSONfunction is correctly implemented and handles all possibleVaultStatusenum values.However, the case clause at lines 83-84 is redundant because of the default clause. Please consider removing it to simplify the code.
Apply this diff to remove the redundant case clause:
- case VaultStatus.UNRECOGNIZED: default: return "UNRECOGNIZED";Tools
Biome
[error] 83-84: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (5 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/protocol/v1/vault.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
- indexer/packages/v4-protos/src/codegen/google/bundle.ts
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/protocol/v1/vault.ts
[error] 60-61: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 61-62: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 83-84: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (12)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/protocol/v1/vault.ts (2)
2-18: LGTM!The
VaultStatusenum is well-defined and follows best practices:
- Each status has a clear description.
- An
UNRECOGNIZEDstatus is included to handle unexpected values.- The default
VAULT_STATUS_UNSPECIFIEDstatus is marked as invalid and unused.
21-37: What is the purpose of theVaultStatusSDKTypeenum?The
VaultStatusSDKTypeenum is identical to theVaultStatusenum. Please clarify the purpose of having this duplicate enum. If it serves a specific purpose, consider adding a comment to explain its usage.indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (4)
60-67: LGTM!The new import statements for protocol, redis, shared, and socks modules follow the correct syntax and naming conventions. Their addition suggests an expansion of functionality within the
dydxprotocolnamespace.
Line range hint
184-385: LGTM!The updated export declarations for various modules maintain a consistent structure and correctly reference the additional imported entities using the spread operator and import aliases. These changes suggest an enhancement of functionality within each module by incorporating new components.
368-377: Significant enhancement: Addition of thevaultmodule.The introduction of the
vaultmodule to thedydxprotocolnamespace represents a significant enhancement to the protocol's functionality related to vault operations. The exported entities within thevaultmodule, including genesis, params, query, share, tx, and vault, follow a consistent naming convention and structure.This addition is likely to interact with other modules within the namespace to provide a cohesive set of features for managing vaults.
386-389: New feature: Addition of theClientFactoryexport.The introduction of the
ClientFactoryexport to thedydxprotocolnamespace suggests the implementation of a factory pattern for creating client instances. The imported entities used in theClientFactoryexport are likely related to LCD, RPC query, and RPC transaction functionality.This addition follows the same structure as other exports within the namespace and may provide a convenient way to instantiate clients with specific configurations or dependencies.
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (6)
1509-1518: LGTM!The
UpsertVaultEventV1interface looks good. The propertiesaddress,clobPairId, andstatusseem appropriate to represent an event emitted when a vault is created or updated.
1521-1530: Looks good!The
UpsertVaultEventV1SDKTypeinterface correctly mirrors theUpsertVaultEventV1interface using the snake_case naming convention for the SDK type.
3832-3838: LGTM!The
createBaseUpsertVaultEventV1function correctly initializes a base instance ofUpsertVaultEventV1with the expected default values for the properties.
3841-3855: Looks good!The
UpsertVaultEventV1.encodefunction correctly encodes an instance ofUpsertVaultEventV1by writing theaddress,clobPairId, andstatusproperties to theWriter.
3857-3885: LGTM!The
UpsertVaultEventV1.decodefunction correctly decodes an instance ofUpsertVaultEventV1by reading theaddress,clobPairId, andstatusproperties from theReaderand returning the decoded message.
3887-3894: Looks good!The
UpsertVaultEventV1.fromPartialfunction correctly creates an instance ofUpsertVaultEventV1from a partial object. It assigns the provided values to the corresponding properties or uses default values if not provided.
Changelist
Add upsert vault event.
Test Plan
Unit tests.
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
dydxprotocolnamespace with the addition of thevaultmodule, improving functionality related to vault operations.Bug Fixes