Possible unreachable code paths
0xAlcibiades opened this issue · 1 comments
0xAlcibiades commented
Adds/updates unit tests for error conditions that weren't being covered. There are a 4 code paths / errors I think may be unreachable. Would love feedback, and if so, I can either remove the check/error itself or update the test to exercise the sad path.
write() L227 — how can a user hit InvalidOption revert?
write() L263 — how can a user hit AlreadyClaimed revert? In other words, how can a user redeem a claim (when now >= expiry) and then write new options to the same contract, when we can't add new options to an expired contract?
redeem() L353 — how can a user have balance != 0 of a claim which is burned? I think this would have to be true for a call to reach this code path, otherwise it's caught during the if (balance != 1) { revert CallerDoesNotOwnClaimId(claimId); } check. Look at the commented-out test on L1177 — in this case, the claim NFT which would have balance == 1 from the 2nd write() call is not the claimId being passed to the 2nd redeem() call. Depending if number 2 and number 3 are actually unreachable, the AlreadyClaimed() error would not be thrown anywhere.
NoClaims error is not being thrown anywhere anymore (only instance I see is a previous usage in valorem-options/lib/valorem-core) — good to remove, or do we need to add a check to exercise() for the case where a user tries to redeem a claim for an option type with no options written, or outstanding ?
Flip-Liquid commented
@0xAlcibiades tagging in here