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

Possible unreachable code paths #97

Closed
0xAlcibiades opened this issue Nov 10, 2022 · 1 comment
Closed

Possible unreachable code paths #97

0xAlcibiades opened this issue Nov 10, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@0xAlcibiades
Copy link
Member

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 ?
@0xAlcibiades 0xAlcibiades self-assigned this Nov 10, 2022
@0xAlcibiades 0xAlcibiades added the bug Something isn't working label Nov 10, 2022
@Flip-Liquid Flip-Liquid self-assigned this Nov 16, 2022
@Flip-Liquid
Copy link
Contributor

@0xAlcibiades tagging in here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants