Skip to content

Add ACL Mapping for Dex Send and Cancel orders #360

Merged
BrandonWeng merged 5 commits into2.0.0betafrom
bweng-dex-deps
Nov 2, 2022
Merged

Add ACL Mapping for Dex Send and Cancel orders #360
BrandonWeng merged 5 commits into2.0.0betafrom
bweng-dex-deps

Conversation

@BrandonWeng
Copy link
Contributor

@BrandonWeng BrandonWeng commented Nov 1, 2022

Describe your changes and provide context

  • Adding the ACL mapping for dex send and cancel orders
  • Also fix the completion signal mapping, we were getting lucky previously with the testing

From Eric's changes: Also updates load testing script to send cancellation orders

Testing performed to validate your change

Mainly unit tests for now - trying to run e2e with loadtest cluster

image

app/app.go Outdated
Comment on lines +973 to +978
if val, ok := channelsMapping[messageIndex]; ok {
channels = val[accessOperation]
} else {
channelsMapping[messageIndex] = make(sdkacltypes.AccessOpsChannelMapping)
}

Copy link
Contributor Author

@BrandonWeng BrandonWeng Nov 2, 2022

Choose a reason for hiding this comment

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

This was a bug, added a unit test to catch this. This resets the channelMapping for each index every time so we really only have one accessOperation channel to signal off. Probably less of a problem when we had much simpler messaging dependencies previously it would happen periodically but now it's consistently happening.

app/app.go Outdated
var channels []chan interface{}
channelsMapping[messageIndex] = make(sdkacltypes.AccessOpsChannelMapping)
if val, ok := channelsMapping[messageIndex]; ok {
channels = val[accessOperation]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to get the channels here? since we're iterating over accessOperation, shouldnt we be starting with a fresh slice each iteration? (aka there shouldnt be an entry for val[accessOperation] right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im gonna remove this if statement entirely, we can move channelsMapping[messageIndex] = make(sdkacltypes.AccessOpsChannelMapping)to the outer loop so it's only instantiated once

"market_order_percentage": "0.8"
},
"message_type": "dex",
"run_oracle": true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: you might want to leave run_oracle false since idt your ansible script (or the one committed in master) has the steps for getting validator keys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted for next PR: #360 (comment)

loadtest/main.go Outdated
Orders: orderPlacements,
ContractAddr: contract,
Funds: amount,
msgType := c.LoadTestConfig.MsgTypeDistr.SampleDexMsgs()
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you rebase your loadtest script onto cherry-pick-parallel instead of 2.0.0beta? theres several changes made there that I'd prefer get included by default instead of rebasing conflicts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im getting a lot of merge conflicts when trying to rebase it on loadtest/main.go, maybe i can just revert the loadtest/changes to 2.0.0beta and make the loadtest/changes against your branch in another PR?

@BrandonWeng BrandonWeng changed the base branch from 2.0.0beta to cherry-pick-parallel November 2, 2022 13:48
@BrandonWeng BrandonWeng changed the base branch from cherry-pick-parallel to 2.0.0beta November 2, 2022 13:51
@BrandonWeng BrandonWeng requested a review from udpatil November 2, 2022 13:54
@BrandonWeng BrandonWeng merged commit b2452a7 into 2.0.0beta Nov 2, 2022
@BrandonWeng BrandonWeng deleted the bweng-dex-deps branch November 2, 2022 13:57
BrandonWeng added a commit that referenced this pull request Nov 2, 2022
* Implement DEX parallelization

* Remove print

* sd

* check out loadtest to 2.0.0beta

* fix

Co-authored-by: Eric Zhu <ericzhu77@gmail.com>
udpatil pushed a commit that referenced this pull request Nov 3, 2022
* Implement DEX parallelization

* Remove print

* sd

* check out loadtest to 2.0.0beta

* fix

Co-authored-by: Eric Zhu <ericzhu77@gmail.com>
udpatil pushed a commit that referenced this pull request Nov 3, 2022
* Implement DEX parallelization

* Remove print

* sd

* check out loadtest to 2.0.0beta

* fix

Co-authored-by: Eric Zhu <ericzhu77@gmail.com>
udpatil added a commit that referenced this pull request Nov 3, 2022
* Revert "Revert "Rebase 2.0.0beta Parallelization onto master" (#357)"

This reverts commit 977c1d1.

* Add ACL Mapping for Dex Send and Cancel orders  (#360)

* Implement DEX parallelization

* Remove print

* sd

* check out loadtest to 2.0.0beta

* fix

Co-authored-by: Eric Zhu <ericzhu77@gmail.com>

* bump version

* Small Refactor for loadtest script (#362)

* sample

* more money

* lint

Co-authored-by: Brandon Weng <18161326+BrandonWeng@users.noreply.github.com>
Co-authored-by: Eric Zhu <ericzhu77@gmail.com>
philipsu522 pushed a commit that referenced this pull request Nov 3, 2022
* Revert "Revert "Rebase 2.0.0beta Parallelization onto master" (#357)"

This reverts commit 977c1d1.

* Add ACL Mapping for Dex Send and Cancel orders  (#360)

* Implement DEX parallelization

* Remove print

* sd

* check out loadtest to 2.0.0beta

* fix

Co-authored-by: Eric Zhu <ericzhu77@gmail.com>

* bump version

* Small Refactor for loadtest script (#362)

* sample

* more money

* lint

Co-authored-by: Brandon Weng <18161326+BrandonWeng@users.noreply.github.com>
Co-authored-by: Eric Zhu <ericzhu77@gmail.com>
masih pushed a commit that referenced this pull request Sep 29, 2025
- To avoid multiple deserialization, as well as adding cached values to
sdk.Tx
- Add new acl constants for evm subprefixes
- Add a new bank send method that doesn't automatically create accounts
unit tests & local sei integration
masih pushed a commit that referenced this pull request Sep 30, 2025
- To avoid multiple deserialization, as well as adding cached values to
sdk.Tx
- Add new acl constants for evm subprefixes
- Add a new bank send method that doesn't automatically create accounts
unit tests & local sei integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants