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

PR for CI test #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

PR for CI test #2

wants to merge 4 commits into from

Conversation

anilhelvaci
Copy link
Collaborator

@anilhelvaci anilhelvaci commented Oct 4, 2024

closes: Agoric/BytePitchPartnerEng#8
closes: Agoric#10111

Description

Currently acceptance tests do not cover auction functionality. When we look at the proposals in https://github.com/Agoric/agoric-3-proposals, there are no tests that check for depositors or bidders payouts from a given auction. There is code for changing auction params here and there, including z:acceptance.

In this PR, we test an auction from start to end. Here's the test case implemented;

  • In USE phase of n:upgrade-next, we place a bid
  • Start executing TEST phase of z:acceptance
  • We push a price for ATOM
  • gov1 deposits 100 IST to ATOM auction book
  • user1 places a bid
  • gov2 places a bid
  • Auction round ends
  • Depositor gets correct payouts
  • All bidders including the one placed in an earlier proposal get correct payouts

Why is this PR a draft?

This pr depends on tools in Agoric#10171, as soon that's approved I'll rebase this one on top of that one.

Security Considerations

None.

Scaling Considerations

Currently this test takes from 4-5 minutes to 12-13 minutes. I can take up to 20 minutes. This is because how the auction params are set. Why not just change the auction params? See Testing Considerations.

Documentation Considerations

None.

Testing Considerations

Testing auctions in a3p have a huge disadvantage since we don't have any tools for controlling how the time advances when running an actual node. I believe Agoric/agoric-3-proposals#179 should be priority as it will unlock very comprehensive test coverage in a3p testing.

Since time is a variable that we cannot control but it also plays a huge role when running our auction tests, we need to wait for it. Another complication is coming from how a3p images are built. Imagine this scenario;

  • chain stopped at t - 3
  • when the chain stopped, nextStartTime of the auction schedule was t - 1
  • auction tests started the chain at t
  • when chain started at t the waker set for the round t - 1 is triggered before our run-test.sh is run
  • the callback of the waker captured the prices then tried to start the round at t - 1 when it is actually at t
  • scheduler realized this and rescheduled the round at t - 1 to start at t + 1
  • round at t + 1 is now the activeStartTime but there's a +1 delay that we have to wait for
  • our test starts depositing and bidding after the price we set is captured by the auctioneer
  • price for activeStartTime is already captured when the t - 1 waker run during the fast forward when chain restarted
  • we need the auction to run with the price we set so we can make sure the long living bid is ended with correct payouts
  • auctioneer will capture new prices at newStartTime
  • so the soonest we can start running out test is at newStartTime
  • we have to wait for +1 (assuming we are still at t) PLUS a whole auction round

This scenario is common when yarn test -m acceptance uses caching as the latest cached layer will have it's state at when it was built. So if you built your cached layer an hour ago and auction start frequency is 10 minutes, you will face this issue.

If the latest cached layer is built less than 15 mins ago (10 mins round time and 5 mins maxLate, auctioneer is okay if round starts less than 5 mins), it very unlikely that you will face this issue("unlikely" because my math there might be wrong.) So I assume we will not face this issue when running in CI as I think all build layers are built from scratch in CI.

Why not update auction params?

Changed params take effect AFTER the next scheduled round. We are only interested in the next scheduled round since it is the soonest we can start depositing and bidding. One way to make the changed params effective for the next scheduled round in our test-acceptance build might be to change them in an earlier proposal USE state which means we have to depend on the global state to carry out our tests which is something I'm not a fan of.

Who places the long living bid?

Again, in order not to depend on global state we create a new user called long-living-bidder. Because other tests might mess with the bidder's wallet history and make querying payouts more complex.

Upgrade Considerations

None.

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.

migrate existing auction related tests to z:acceptance acceptance test of bids surviving upgrades
1 participant