Handle underfunded claims #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lalexgap
commented
May 7, 2021
| @@ -14,7 +14,11 @@ export function claim( | |||
| ) { | |||
| if (initialTargetOutcome.length !== initialHoldings.length) throw Error; | |||
| const updatedTargetOutcome: Exit = []; | |||
| let updatedHoldings = initialHoldings; | |||
| // We want to create a clone of the original values to prevent mutation | |||
| const updatedHoldings = initialHoldings.map((h) => BigNumber.from(h)); | |||
There was a problem hiding this comment.
I think previously we were mutating the values passed in
lalexgap
commented
May 7, 2021
geoknee
reviewed
May 8, 2021
Comment on lines
+71
to
+74
| const guarantee = createGuarantee([ | ||
| ["C2", "0x03", ["A"]], | ||
| ["C1", "0x0A", ["A", "I", "B"]], | ||
| ]); |
There was a problem hiding this comment.
I like this notation a lot, it's easy to parse the important information.
Any reason why you chose C2 as the higher priority and C1 as the lower priority (just wondering about notation)?
There was a problem hiding this comment.
No strong reason.
In my mind, C1 is the "main" channel (with allocations to A,I,B) and we're using C2 ahead of it to test the underfunded claim scenario. It keeps it consistent-ish with other tests that use C1 as the only channel.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates the
claimmethod to take into account the order ofguarantees. We iterate through guarantees that occur before the target one and tally the amount of funds reserved by guarantees.The amount on the
guaranteeis now used to store the funding available for the guarantee.We also now return a
updatedGuaranteeOutcomethat has the amount adjusted after the funds have been claimed.Currently there are only typescript versions of the tests/changes but this should be pretty easy to copy over to solidity.