-
Notifications
You must be signed in to change notification settings - Fork 30
chore(marketplace): use canMarkProofAsMissing #1188
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
Conversation
9a0e19c
to
fe51f53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple suggestions, but no blockers, so will approve now 👍
test "can check whether a proof cannot be marked as missing before a proof is required": | ||
let slotId = slotId(request, slotIndex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that each test starts off at the exact same point and assuming that the block hashes will always be deterministic, this test SHOULD work every time, but there may be a chance that there is a proof required as soon as the slot is filled.
check (await market.canProofBeMarkedAsMissing(slotId, missingPeriod)) == true | ||
check (await market.canMarkProofAsMissing(slotId, missingPeriod)) == true | ||
|
||
test "can check whether a proof cannot be marked as missing when the slot is free": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add a test that checks if a proof is required before the slot is filled (also free)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's really useful, because if a slot isn't filled, we are not able to call waitUntilProofRequired
which is almost the same situation as in the test "can check whether a proof cannot be marked as missing before a proof is required", no ?
fe51f53
to
909d801
Compare
909d801
to
3634d86
Compare
3634d86
to
f34a12a
Compare
This PR fixes #1153 by using a new function in the contract, canProofBeMarkedAsMissing.
Since this function is a view and does not transfer reward funds to the validator, we should no longer encounter the ERC20: transfer to the zero address error.