ife stop eating wrong side bond #585
Conversation
boolafish
changed the title
| { | ||
| for (uint16 i = 0; i < PaymentTransactionModel.MAX_INPUT_NUM(); i++) { | ||
| PaymentExitDataModel.WithdrawData memory withdrawal = exit.inputs[i]; | ||
| if (token == withdrawal.token && exit.isInputPiggybacked(i)) { |
There was a problem hiding this comment.
this might be up to the wording (and possibly require a clarifying comment), but I'm assuming here that invalid and challenged piggybacks won't return the bond, right?
There was a problem hiding this comment.
Thanks for pointing this out. I just double checked the challenge piggyback code. So if it challenged, it would return the bond immediately and delete the piggyback flag. So this implementation is fine.
There was a problem hiding this comment.
But I definitely have the wrong impression in my mind when making the code change....I thought challenge piggyback would end up changing the piggyback bond owner.
There was a problem hiding this comment.
OK, maybe consider leaving a comment about challenged piggybacks - that this is taken care of b/c isInputPiggybacked returns false.
| @@ -464,6 +465,14 @@ contract('PaymentProcessInFlightExit', ([_, ifeBondOwner, inputOwner1, inputOwne | |||
| expect(postBalance).to.be.bignumber.equal(expectedBalance); | |||
| }); | |||
|
|
|||
| it('should return piggyback bond to the output owner', async () => { | |||
There was a problem hiding this comment.
is there testing that a challenged invalid piggyback doesn't return the bond? Also are both canonicity-cases covered in here?
This commit change the behavior to be returning piggyback bond no matter it is on the right side of canoncity or not. issue: #584
boolafish
force-pushed
the
boolafish/ife_stop_eating_wrong_side_bond
branch
from
9b0d5b9
to
15dffd9
Compare
plasma_framework/contracts/src/exits/payment/controllers/PaymentProcessInFlightExit.sol
Outdated
Show resolved
Hide resolved
plasma_framework/contracts/src/exits/payment/controllers/PaymentProcessInFlightExit.sol
Outdated
Show resolved
Hide resolved
by PR review suggestion Co-Authored-By: Kevin Sullivan <4653170+kevsul@users.noreply.github.com>
Also format the README file a bit.
- run `npm version patch -m` - update changelog
for PR review Co-Authored-By: Kevin Sullivan <4653170+kevsul@users.noreply.github.com>
plasma_framework/contracts/src/exits/payment/controllers/PaymentProcessInFlightExit.sol
Outdated
Show resolved
Hide resolved
Although the inputs and outputs storage is defined to be exactly the max size of payment tx inputs/outputs. However, using the length itself makes the code more self explaining.
Note