[WIP] Adding Inflight tx exits #132

nanspro · 2019-01-28T08:33:29Z

Implementing Limbo(Inflight tx) Exits
Fixes #86

nanspro · 2019-01-28T14:41:16Z

test/exitHandler.js

+        // transaction hash + constant signed by alice
+        const signature = (transferTx + c).sign(alicePriv);
+
+        const inTxData = Tx.parseToParams(transferTx);


We can't calculate transferProof because period is not getting submitted(transferTx is inflight) so i am parsing it to data and then passing it to function and in our contract we can parse this data to obtain the transaction.

possible, even simpler would be transferTx.hex().

nanspro · 2019-01-28T14:44:09Z

test/exitHandler.js

+        const inTxData = Tx.parseToParams(transferTx);
+
+        // Bob will start the exit for Alice
+        await exitHandler.startLimboExit(inTxData, signature, outputIndex, 


Who will start the limbo exit for tx A->B? A or B?
(Current) B starts a limbo exit by submitting all data A would submit if A were exiting from the coin, as well as the signature received and the in-flight transaction.. All data A would submit will be the depositTx and depositProof.

nanspro · 2019-01-28T15:09:33Z

test/exitHandler.js

+          (b, tx) => b.addTx(tx),
+          new Block(33)
+        );
+        prevRoot = period.merkleRoot();


I need spendProof (Proof that bob sent the money to charlie) so that i can pass it to challengeLimbo and validate it there. Can i create a block simply and not publish it to Bridge so that i can get period.

nanspro · 2019-01-28T15:12:11Z

test/exitHandler.js

+        const depositProof = period.proof(depositTx);
+        const outputIndex = 1;
+        const exitStake = exitHandler.exitStake();
+        const signature = (transferTx + c).sign(alicePriv);


A sends B a signature on the hash of the in-flight transaction plus some special constant.
Need to fix it

nanspro · 2019-01-28T15:16:41Z

contracts/ExitHandler.sol

+    bytes32 depositTxHash;
+    (txPos, depositTxHash,) = TxLib.validateProof(96, depositProof);
+    require(
+      depositTxHash == transferTx.ins[*].outpoint.hash, 


Was supposed to pass _inputIndex in the function and put it here but i was looking for some other way to verify this.

johannbarbie · 2019-01-29T08:53:09Z

contracts/ExitHandler.sol

@@ -181,6 +181,64 @@ contract ExitHandler is DepositHandler {
    exitDuration = _exitDuration;
  }

+  function startLimboExit(
+    bytes memory limboTxData, bytes32 signature, uint8 _outputIndex,


@nanspro i think you have been looking at the wrong specification. there should not be any signatures for more viable plasma. can you make sure that you tried to implement this: https://ethresear.ch/t/more-viable-plasma/2160

nanspro · 2019-02-05T02:18:11Z

@johannbarbie i've updated the PR(still need to update test cases though), could you check it out once and give me a feedback upon it.
I still need to do some more things in core contract (labeled as TODO in comments).

johannbarbie · 2019-02-05T07:17:21Z

could you check it out once and give me a feedback upon it.

👀

johannbarbie · 2019-02-05T07:21:01Z

contracts/ExitHandler.sol

+
+    bytes32 inTxHash = keccak256(inTxData);
+
+    // for now assume outputIndex 0


what should this be later? youngest input?

If a tx has multiple inputs and multiple outputs then there will be multiple utxo's so right now i am handling one but a tx might have more so i how should i handle that scenario?
One possible soln: if a tx has multiple utxo's then i can write a combine function which will get me combined priority for the tx (i can use getERC20ExitPriority to get individual utxo priority)

Can i use youngest input in any way for this?

johannbarbie · 2019-02-05T07:22:39Z

contracts/ExitHandler.sol

+      output: outputs,
+      input: inputs,
+      finalized: false,
+      exitor: msg.sender,


does there need to be any special relation between msg.sender and the transaction? signer of inputs?

no there's actually no need for storing it with tx, i thought we might need it to check if same address is initiating large amount of exits but i think keeping exitStake handles this problem
Will remove it

johannbarbie · 2019-02-05T07:25:31Z

contracts/ExitHandler.sol

+  function challengeLimboExitByOutputSpend(
+    bytes32 exitId,
+    bytes inTxData, uint8 inOutputNo,
+    bytes competingTxData, bytes proof, uint8 competingInputNo, uint32 blockNumber) 


i think here you should borrow from our existing proof structure (bytes32[]), which will allow you to read the blockHeight from the bridge

Yeah i am comparing blockHeight of inTx and spendTx to see who comes first.

johannbarbie · 2019-02-05T07:27:13Z

contracts/ExitHandler.sol

+    TxLib.Tx memory transferTx = Tx.parseTx(inTxData);
+
+    // [TODO]check if this tx is included or not
+    // TxLib.Tx memory includedTx = checkForValidityAndInclusion(blockNumber, includedTxData, includedProof);


wouldn't you need to pass an inclusion proof here? i think you can copy from the other exit functions: https://github.com/leapdao/leap-contracts/blob/master/test/exitHandler.js#L134-L136

johannbarbie · 2019-02-05T07:27:59Z

contracts/ExitHandler.sol

+
+  function respondInputSpendChallenge(
+    bytes32 exitId, bytes inTxData, uint8 inInputNo,
+    bytes includedTxData, bytes includedProof, uint8 includedOutputNo, uint32 blockNumber


here also use bytes32[] for inclusion proof and make use of leap-core and txLib.sol.

johannbarbie

@nanspro you are on the right way. i think figuring out the inclusion proofs is the biggest gap left. awesome work 👍

troggy · 2019-02-15T16:54:05Z

contracts/ExitHandler.sol

+    uint32 timestamp;
+
+    // priority based on youngestInput
+    if (_youngestInputProof.length > 0) {


unnecessary check? We always need youngestInputProof, no? Otherwise, I guess it will fail on line 171 (validateProof)

troggy · 2019-02-15T17:13:29Z

contracts/ExitHandler.sol

+
+  function challengeLimboExitByInclusionProof(
+    bytes32 exitId, bytes memory inTxData,
+    uint8 InputNo, bytes32[] memory _youngestInputProof, bytes32[] memory  _inclusionProof) 


_youngestInputProof is not used in any of these

troggy

I haven't done a complete review. This looks like a nice effort, however couple of my findings make me think that the whole implementation is not working correct yet. Having a working set of unit tests will definitely help here.

I will continue review once the tests are done, so I can make sure I do my best work here.

troggy · 2019-02-15T17:23:11Z

contracts/ExitHandler.sol

+      for(uint8 i=0; i < currentExit.output.length; i++){
+        LimboOut memory out = currentExit.output[i];
+        if (out.exitable){
+          amount = piggybackStake + out.amount;


piggybackStake is in wei and out.amount is in tokens. Separate transfers needed here

troggy · 2019-02-15T17:23:18Z

contracts/ExitHandler.sol

+      for(uint8 i=0; i < currentExit.input.length; i++){
+        LimboIn memory input = currentExit.input[i];
+        if (input.exitable){
+          amount = piggybackStake + input.amount;


piggybackStake is in wei and input.amount is in tokens. Separate transfers needed here

troggy · 2019-02-15T17:25:43Z

contracts/ExitHandler.sol

+      uint32 prevHeight = bridge.periods[_prevProof[0]].height;
+
+      require(blockHeight < prevHeight);
+      tokens[transferTx.outs[0].color].addr.transferFrom(address(this), msg.sender, challengeStake);


challengeStake is in ether, so it should transfer ether here. Also the destination is not correct: we are returning stake of the previous challenger, so it should transfer to prevChallenge.owner

troggy · 2019-02-15T17:26:51Z

contracts/ExitHandler.sol

+
+    require(inTxHash == txHash, "Invalid inclusion proof");
+
+    limboExit.isValid = false;


should not we check that provided tx has any connection to challenged exit?

johannbarbie · 2019-02-21T14:23:02Z

@nanspro how are things here? :)

nanspro changed the title ~~Adding some basic fucntions~~ Adding some basic functions Jan 28, 2019

nanspro changed the title ~~Adding some basic functions~~ Adding Inflight tx exits Jan 28, 2019

nanspro force-pushed the master branch from 6e7dc49 to 3ae5bc7 Compare January 28, 2019 09:47

nanspro changed the title ~~Adding Inflight tx exits~~ [WIP] Adding Inflight tx exits Jan 28, 2019

nanspro commented Jan 28, 2019

View reviewed changes

johannbarbie reviewed Jan 29, 2019

View reviewed changes

nanspro force-pushed the master branch 3 times, most recently from 0ffcbbf to d495814 Compare February 5, 2019 01:54

johannbarbie reviewed Feb 5, 2019

View reviewed changes

nanspro requested review from eezcjkr and removed request for eezcjkr February 10, 2019 02:26

nanspro force-pushed the master branch 2 times, most recently from f5f89cb to ec05c37 Compare February 12, 2019 00:43

troggy reviewed Feb 15, 2019

View reviewed changes

troggy requested changes Feb 15, 2019

View reviewed changes

Adding basic intended functionality and tests

3e1d164

nanspro added 4 commits February 27, 2019 15:53

Handling challenges and resolvers

22ef038

Adding proofs to validate tx flow

7427aed

Everything except working tests

91c7d73

Code not compiling aaaaaaaaaaaa

37ef21d

nanspro force-pushed the master branch from 100fa42 to 37ef21d Compare February 27, 2019 10:24

New thoughts

5545966

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Adding Inflight tx exits #132

[WIP] Adding Inflight tx exits #132

nanspro commented Jan 28, 2019 •

edited

Loading

nanspro Jan 28, 2019

johannbarbie Feb 5, 2019

nanspro Jan 28, 2019

nanspro Jan 28, 2019

nanspro Jan 28, 2019

nanspro Jan 28, 2019

johannbarbie Jan 29, 2019

nanspro commented Feb 5, 2019

johannbarbie commented Feb 5, 2019

johannbarbie Feb 5, 2019

nanspro Feb 10, 2019 •

edited

Loading

nanspro Feb 10, 2019

johannbarbie Feb 5, 2019

nanspro Feb 10, 2019

johannbarbie Feb 5, 2019

nanspro Feb 10, 2019

johannbarbie Feb 5, 2019

johannbarbie Feb 5, 2019

johannbarbie left a comment

troggy Feb 15, 2019

troggy Feb 15, 2019

troggy left a comment

troggy Feb 15, 2019

troggy Feb 15, 2019

troggy Feb 15, 2019

troggy Feb 15, 2019

johannbarbie commented Feb 21, 2019


		bytes32 inTxHash = keccak256(inTxData);

		// for now assume outputIndex 0


		require(inTxHash == txHash, "Invalid inclusion proof");

		limboExit.isValid = false;

[WIP] Adding Inflight tx exits #132

Are you sure you want to change the base?

[WIP] Adding Inflight tx exits #132

Conversation

nanspro commented Jan 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanspro commented Feb 5, 2019

johannbarbie commented Feb 5, 2019

Choose a reason for hiding this comment

nanspro Feb 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johannbarbie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

troggy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johannbarbie commented Feb 21, 2019

nanspro commented Jan 28, 2019 •

edited

Loading

nanspro Feb 10, 2019 •

edited

Loading