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

Jump to bottom

Feature/2124 implement ratelimit #2186

Merged
merged 5 commits into from
Sep 3, 2020
Merged

Feature/2124 implement ratelimit #2186

merged 5 commits into from
Sep 3, 2020

Conversation

qustavo
Copy link

@qustavo qustavo commented Sep 3, 2020

No description provided.

@vega-ci-bot
Copy link
Collaborator

system-tests failed.

No test summary found

1 similar comment
@vega-ci-bot
Copy link
Collaborator

system-tests failed.

No test summary found

@edd edd mentioned this pull request Sep 3, 2020
Closed
10 tasks
@vega-ci-bot
Copy link
Collaborator

system-tests failed.

No test summary found

@edd edd mentioned this pull request Sep 3, 2020
Closed
@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/tradesTests.py .......                         [100%]

@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/positionsTests.py .                            [100%]
../../grpcTesting/trading/tradesTests.py .......                         [100%]

1 similar comment
@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/positionsTests.py .                            [100%]
../../grpcTesting/trading/tradesTests.py .......                         [100%]

EVODelavega
EVODelavega reviewed Sep 3, 2020
View reviewed changes
blockchain/ratelimit/ratelimit.go Outdated
@@ -57,25 +55,14 @@ func (r *Rates) NextBlock() {
return
}

for key := range r.entries {
for key, _ := range r.entries {
Copy link
Contributor

Choose a reason for hiding this comment

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

why add the , _? Doesn't the linter complain about this

Copy link
Author

Choose a reason for hiding this comment

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

yeah, I guess this was introduced when I reverted the previous whitelist implementation

EVODelavega
EVODelavega reviewed Sep 3, 2020
View reviewed changes
blockchain/blockchain.go Outdated
@@ -40,6 +40,11 @@ type GenesisHandler interface {
OnGenesis(genesisTime time.Time, appState []byte, validatorsPubkey [][]byte) error
}

//go:generate go run github.com/golang/mock/mockgen -destination mocks/validator_topology_mock.go -package mocks code.vegaprotocol.io/vega/blockchain ValidatorTopology
type ValidatorTopology interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only to pass the dependency on to the abci application, why declare the interface twice, instead of using tm.ValidatorTopology?

Copy link
Author

Choose a reason for hiding this comment

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

👍

@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/positionsTests.py .                            [100%]
../../grpcTesting/trading/tradesTests.py .......                         [100%]

@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/positionsTests.py F                            [100%]
 testFullyFilled[orderparam0] 
../../grpcTesting/trading/tradesTests.py .......                         [100%]

@vega-ci-bot
Copy link
Collaborator

system-tests failed.

=== Test summary ===
../../grpcTesting/governance/newAssetTests.py EEEE                       [100%]
 ERROR at setup of testDepositBuiltInAsset 
 ERROR at setup of testWithdrawEnactedBuiltInAsset 
 ERROR at setup of testWithdrawAnyBuiltInAsset 
 ERROR at setup of testSubmitOrderNewMarketNewAsset 
../../grpcTesting/markets/marketsTests.py ..............                 [100%]
../../grpcTesting/orders/cancelOrdersTests.py ...                        [100%]
../../grpcTesting/orders/generalOrdersTests.py .............             [100%]
../../grpcTesting/orders/limitOrdersTests.py .......                     [100%]
../../grpcTesting/orders/marketOrdersTests.py ........                   [100%]
../../grpcTesting/orders/negativeOrdersTests.py ...........              [100%]
../../grpcTesting/trading/positionsTests.py .                            [100%]
../../grpcTesting/trading/tradesTests.py .......                         [100%]

@qustavo qustavo force-pushed the feature/2124-implement-ratelimit branch from 692ffef to 4867e96 Compare September 3, 2020 15:12
@qustavo qustavo merged commit d440741 into develop Sep 3, 2020
@jeremyletang jeremyletang deleted the feature/2124-implement-ratelimit branch March 5, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EVODelavega EVODelavega EVODelavega left review comments

@jeremyletang jeremyletang jeremyletang approved these changes

@ashleyvega ashleyvega Awaiting requested review from ashleyvega

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@qustavo @vega-ci-bot @EVODelavega @jeremyletang