Record next challenge epoch. Extract CID to library. #38
Conversation
contracts/test/PDPService.t.sol
Outdated
| } | ||
|
|
||
| // TODO: test bad inputs to addRoot | ||
| // TODO: test removing roots, good and bad inputs |
There was a problem hiding this comment.
There already exist tests removing roots. Though I agree we don't have as much testing as I'd like I thought that this fell more under fuzzing than unit tests. What else are you specifically looking for here?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think I get it, you'd like a more thorough check of all state variable changed by adding / removing. Can do
| @@ -184,6 +197,11 @@ contract PDPService { | |||
| totalDelta += removeOneRoot(setId, rootIds[i]); | |||
| } | |||
| proofSetLeafCount[setId] -= totalDelta; | |||
| // Clear next challenge epoch if the set is now empty. | |||
| // It will be re-set when new data is added. | |||
| if (proofSetLeafCount[setId] == 0) { | |||
There was a problem hiding this comment.
I think we should keep a counter of live roots and consult that both for the needsChallengeEpoch bool above and for this one. I think it makes sense to do here. But if you'd rather defer that that's ok.
There was a problem hiding this comment.
I'm not convinced yet, so will defer.
anorth
force-pushed
the
anorth/challenge-epoch
branch
from
59513a8 to
26ca0c3
Compare
anorth
force-pushed
the
anorth/challenge-epoch
branch
from
26ca0c3 to
37bb48b
Compare
This is a prefactor extracted from my work implementing
provePosession. In addition to adding the challenge epoch, I realised there is insufficient testing for the basic logic around adding and removing pieces, and resolving all this was going to balloon the PR adding proving (for which the testing will be quite involved).I haven't added all the necessary tests here, but it would be great @ZenGround0 if you could follow on from this and fill out those workflow tests.