Increase ShortBlockWindow to 30 due to decreased blocktimes #3226
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughShort-block timing and related test GTB values were increased: Changes
Sequence Diagram(s)(No sequence diagram included: changes are numeric constants/config updates without new control-flow paths.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
protocol/x/clob/e2e/app_test.go(3 hunks)protocol/x/clob/e2e/rate_limit_test.go(3 hunks)protocol/x/clob/types/constants.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-04-15T16:58:46.335Z
Learnt from: anmolagrawal345
Repo: dydxprotocol/v4-chain PR: 2780
File: protocol/x/clob/keeper/twap_order_state_test.go:191-209
Timestamp: 2025-04-15T16:58:46.335Z
Learning: For TWAP orders, message validation in message_place_order.go handles edge cases with comprehensive checks for parameters like durations and intervals, making it unnecessary to test invalid parameters in twap_order_state_test.go which only receives pre-validated inputs.
Applied to files:
protocol/x/clob/e2e/app_test.goprotocol/x/clob/e2e/rate_limit_test.go
📚 Learning: 2024-11-15T16:17:29.092Z
Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2551
File: protocol/x/clob/types/expected_keepers.go:86-90
Timestamp: 2024-11-15T16:17:29.092Z
Learning: The function `GetCrossInsuranceFundBalance` in `protocol/x/clob/types/expected_keepers.go` already existed and was just moved in this PR; changes to its error handling may be out of scope.
Applied to files:
protocol/x/clob/e2e/app_test.go
📚 Learning: 2025-04-15T16:57:55.413Z
Learnt from: anmolagrawal345
Repo: dydxprotocol/v4-chain PR: 2780
File: protocol/x/clob/keeper/twap_order_state.go:0-0
Timestamp: 2025-04-15T16:57:55.413Z
Learning: TWAP order parameter validation (for interval, duration, and price tolerance) is handled in the message_place_order.go file through the ValidateBasic method, which ensures parameters are within acceptable ranges and that duration is a multiple of interval.
Applied to files:
protocol/x/clob/e2e/app_test.go
🧬 Code graph analysis (2)
protocol/x/clob/e2e/app_test.go (4)
protocol/x/clob/types/message_place_order.go (1)
NewMsgPlaceOrder(33-37)protocol/testutil/app/order.go (1)
MustScaleOrder(68-120)protocol/x/clob/types/message_cancel_order.go (1)
NewMsgCancelOrderShortTerm(19-25)protocol/x/clob/types/message_batch_cancel.go (1)
NewMsgBatchCancel(13-23)
protocol/x/clob/e2e/rate_limit_test.go (1)
protocol/x/clob/e2e/app_test.go (3)
PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB47(81-90)CancelOrder_Alice_Num0_Id0_Clob0_GTB47(178-185)BatchCancel_Alice_Num0_Clob0_1_2_3_GTB47(278-287)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: build-and-push-mainnet
- GitHub Check: benchmark
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: golangci-lint
- GitHub Check: test-coverage-upload
- GitHub Check: build
- GitHub Check: liveness-test
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: build-and-push-testnet
- GitHub Check: test-race
- GitHub Check: container-tests
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (4)
protocol/x/clob/e2e/rate_limit_test.go (1)
709-709: Test updates are mathematically consistent with the new ShortBlockWindow.The GTB value updates from 27 to 47 correctly reflect the increased window (27 = 20 + 7; 47 = 40 + 7). These values test replay attack protection by exceeding the validation window. The changes are correct assuming
ShortBlockWindow = 40is the intended value.Also applies to: 723-723, 737-737
protocol/x/clob/e2e/app_test.go (3)
81-90: Test fixture correctly updated to reflect new ShortBlockWindow.The
PlaceOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTB47fixture (renamed from GTB27) withGoodTilBlock: 47is correctly updated to test values beyond the new validation window. The changes are consistent with the increased ShortBlockWindow.
178-185: Test fixture correctly updated.The
CancelOrder_Alice_Num0_Id0_Clob0_GTB47fixture properly reflects the new GTB value of 47 for testing boundary conditions.
278-287: Test fixture correctly updated.The
BatchCancel_Alice_Num0_Clob0_1_2_3_GTB47fixture properly reflects the new GTB value of 47 for testing batch cancel scenarios beyond the validation window.
|
@mergify backport release/protocol/v9.5.x |
✅ Backports have been created
|
Signed-off-by: Shrenuj Bansal <[email protected]> Co-authored-by: Kefan Cao <[email protected]> (cherry picked from commit adf3ad4)
…3226) (#3262) Signed-off-by: Shrenuj Bansal <[email protected]> Co-authored-by: shrenujb <[email protected]> Co-authored-by: Kefan Cao <[email protected]>
Changelist
Blocktimes were reduced from 1s to ~0.6s which decreased the short term order block window
Increase the ShortBlockWindow to 40 to account for the decreased blocktime.
Also double the mempool size to allow for more orders to live in mempool and change TTL to correspond with shortBlockWindow.
Test Plan
[Describe how this PR was tested (if applicable)]
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