Remove internal _claim map

[Flip-Liquid]

drop the claim() accessor from the ABI. Underlying() returns all necessary information for determining the value of an options position.

[codecov-commenter]

Codecov Report

Merging #141 (d3eff32) into audit-fixes (aa5b23f) will increase coverage by 0.07%. The diff coverage is 95.00%.

@@               Coverage Diff               @@
##           audit-fixes     #141      +/-   ##
===============================================
+ Coverage        88.69%   88.76%   +0.07%     
===============================================
  Files                2        2              
  Lines              345      365      +20     
  Branches            52       54       +2     
===============================================
+ Hits               306      324      +18     
- Misses              30       31       +1     
- Partials             9       10       +1     

Impacted Files	Coverage Δ
src/OptionSettlementEngine.sol	98.31% <95.00%> (-0.77%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[neodaoist]

Nice. I think it's getting in a good way now. Feedback:

1. I really like how you renamed some local vars and named the functions _getExercisedAmountsForClaim and _getExercisedAmountsForClaimIndex — and so now because the only caller of _getAmountExercised is _getExercisedAmountsForClaimIndex, should we just collapse it into _getExercisedAmountsForClaimIndex?
2. Move claim() back to right after option() (in interface and concrete impl)
3. Is there a missing check in claim(), where L147 could instead use tokenType() check? (BTW which would require this to be public not external but that's okay imo). IE, couldn't someone encode a tokenID with a valid, initialized option key but a bunk claim num, and then get weirdness coming back in the OptionLotClaim struct?
4. We should do a NatSpec pass on all these internal fns, but not blocking PR for me — we have #137 for that

[Flip-Liquid]

Nice. I think it's getting in a good way now. Feedback:

1. I really like how you renamed some local vars and named the functions _getExercisedAmountsForClaim and _getExercisedAmountsForClaimIndex — and so now because the only caller of _getAmountExercised is _getExercisedAmountsForClaimIndex, should we just collapse it into _getExercisedAmountsForClaimIndex?
2. Move claim() back to right after option() (in interface and concrete impl)
3. Is there a missing check in claim(), where L147 could instead use tokenType() check? (BTW which would require this to be public not external but that's okay imo). IE, couldn't someone encode a tokenID with a valid, initialized option key but a bunk claim num, and then get weirdness coming back in the OptionLotClaim struct?
4. We should do a NatSpec pass on all these internal fns, but not blocking PR for me — we have Review all NatSpec for simplicity and clarity #137 for that

Addressed 1 & 2, good feedback. For 3, the option lot claim returned would just be zeroed out, with unredeemed == false, which I think is ok

[neodaoist]

Nice. I think it's getting in a good way now. Feedback:

1. I really like how you renamed some local vars and named the functions _getExercisedAmountsForClaim and _getExercisedAmountsForClaimIndex — and so now because the only caller of _getAmountExercised is _getExercisedAmountsForClaimIndex, should we just collapse it into _getExercisedAmountsForClaimIndex?
2. Move claim() back to right after option() (in interface and concrete impl)
3. Is there a missing check in claim(), where L147 could instead use tokenType() check? (BTW which would require this to be public not external but that's okay imo). IE, couldn't someone encode a tokenID with a valid, initialized option key but a bunk claim num, and then get weirdness coming back in the OptionLotClaim struct?
4. We should do a NatSpec pass on all these internal fns, but not blocking PR for me — we have Review all NatSpec for simplicity and clarity #137 for that

Addressed 1 & 2, good feedback. For 3, the option lot claim returned would just be zeroed out, with unredeemed == false, which I think is ok

I think reverting with InvalidClaim is better, but I understand what you're saying. LGTM

[0xAlcibiades]

Please address the 2 sm0l feedbacks and squash merge @Flip-Liquid.