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.

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

Initial stress test for liquidation auction #159

Merged
merged 12 commits into from
Jun 21, 2021
Merged

Conversation

utdemir
Copy link
Contributor

@utdemir utdemir commented Jun 18, 2021

This PR adds a new e2e test called LiquidationsStressTest which creates a thousand liquidation slices.

It mainly tests the push_back function, and it seems to be well below the gas limit since we're triggering 40 liquidations as a bulk operation. We should add similar tests for other AVL functions too (split and del comes to mind).

While doing that, I had to implement a few other minor changes:

  • On this test, we are running a patch version of Checker on which the protected index tracks the actual index much more closely, in order to be able to trigger liquidations quickly. To use this:
CHECKER_DIR=$(nix-build -A michelson --arg e2eTestsHack true) python e2e/main.py LiquidationsStressTest

When we end up in a state where our E2E tests run reliably, we should add this to the CI.

  • Our 'ptr' type is now a 'nat' instead of an 'int'. I just felt like it suits more since it always is positive.
  • Our 'liquidation_auction_place_bid' function now expects an 'auction_id'
  • We have a 'current_liquidation_auction_minimum_bid' view, which also returns the auction id.

Copy link
Contributor

@gkaracha gkaracha left a comment

Choose a reason for hiding this comment

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

LGTM! 👌 I left a few suggestions but apart from the one relating to the potentially failing assertion the rest could be done later as well. It's great to finally have a liquidation test, thanks! 😄

src/ligo.mli Outdated Show resolved Hide resolved
src/ligo.ml Outdated Show resolved Hide resolved
tests/testChecker.ml Outdated Show resolved Hide resolved
src/checker.ml Outdated Show resolved Hide resolved
e2e/main.py Outdated Show resolved Hide resolved
e2e/main.py Outdated Show resolved Hide resolved
Comment on lines +302 to +304
# The return value is supposed to be annotated as "auction_id" and "minimum_bid", I
# do not know why we get these names. I think there is an underlying pytezos bug
# that we should reproduce and create a bug upstream.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, that's weird indeed 🤔 Not sure if it's a pytezos bug; it reminds me of #81.

Copy link
Contributor

@dorranh dorranh left a comment

Choose a reason for hiding this comment

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

Awesome! Just a couple of small comments but otherwise LGTM as well. It would be cool to add some profiling in this test so that we can see the trend in gas costs with successive calls. I'm happy to work on that next week as a separate PR though.

default.nix Show resolved Hide resolved
e2e/main.py Outdated Show resolved Hide resolved
e2e/main.py Outdated Show resolved Hide resolved
src/checker.ml Show resolved Hide resolved
@dorranh dorranh merged commit 543f124 into master Jun 21, 2021
@purcell purcell deleted the ud/stress-test-init branch June 22, 2021 02:22
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.

3 participants