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

E2E Testing for FIP8 & FIP13 #6157

Closed
Tracked by #6185
Stebalien opened this issue Apr 30, 2021 · 7 comments · Fixed by #6526
Closed
Tracked by #6185

E2E Testing for FIP8 & FIP13 #6157

Stebalien opened this issue Apr 30, 2021 · 7 comments · Fixed by #6526
Assignees
Labels
kind/test Kind: Test P1 P1: Must be resolved

Comments

@Stebalien
Copy link
Member

Stebalien commented Apr 30, 2021

FIP8 and FIP13 will allow batched pre-commits and batched prove-commits. This process needs end-to-end testing at scale (100s of sectors, test match batch sizes).

  1. We need unit/integration tests to verify pre/prove-commit batching.
    1. What happens when a single sector fails?
  2. We need to test this in a dev-net.
@Stebalien Stebalien self-assigned this Apr 30, 2021
@BigLep
Copy link
Member

BigLep commented Apr 30, 2021

Discussion notes:

  1. Scale
  • 100's of sectors all at once
  1. Assertions
  • What happens if 1 of 100 sectors fail
  • Pipeline is able to notice that we don't have enough sectors in a batch and thus wait.

@jennijuju jennijuju added P1 P1: Must be resolved and removed Epic labels May 4, 2021
@jennijuju jennijuju mentioned this issue May 4, 2021
80 tasks
@BigLep BigLep assigned magik6k and unassigned Stebalien May 17, 2021
@BigLep
Copy link
Member

BigLep commented May 20, 2021

@magik6k and @vyzo : I want to make sure we're clear about the testing work for FIP8 and 13. Specifically I would like to know what we expect tested, that we have owners, and that we're tracking it (whether as a checklist or separate issues). I took a first stab below based on what I saw in this issue and in PR #6235. Please correct and ideally set as the subject of this issue or another.

In addition to making sure we don't lose track of anything, it'll be important as we update our estimates of the remaining work on 2021-05-20.

Item Comment Status
Unit tests for CommitBatcher Owner: @magik6k | Issue/PR: #6432 Done
Unit tests for PrecommitBatcher Owner: @magik6k | Issue/PR: #6432 Done
Precommit Batching integration test Owner: @mention | Issue/PR: #XXXX
Commit Batching integration test Owner: @mention | Issue/PR: #XXXX
Scale testing This is automated verification that we can handle 100's of sectors

Owner: @mention | Issue/PR: #XXXX

In batching unit/integration tests we'd want to check:

  • Happy path
  • One/multiple sector expires
  • Deal within a sector expires
  • Batch fails (e.g., out of gas)
  • One/many porep fails (commit)
  • that we always recover from those correctly

@magik6k
Copy link
Contributor

magik6k commented May 25, 2021

(updated the comment above to have a better list of what needs to be done)

@arajasek
Copy link
Contributor

#6432 adds unit tests for both the precommit and commit batchers

@BigLep BigLep linked a pull request Jun 18, 2021 that will close this issue
@arajasek
Copy link
Contributor

@BigLep
Copy link
Member

BigLep commented Jun 23, 2021

I'm reopening this because I'm not seeing the integ tests PR created or merged (from looking https://github.com/filecoin-project/lotus/pulls?q=is%3Apr+sort%3Aupdated-desc+ ), but let me know if I have that wrong. I don't want to blur what "done" means.

@arajasek
Copy link
Contributor

Tested on mainnet ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/test Kind: Test P1 P1: Must be resolved
Projects
None yet
5 participants