Quasar Fast Exit: Implement withdrawal the happy path #723

souradeep-das · 2020-12-02T09:23:07Z

Overview

This includes-

Happy Withdrawal path with challenges
Updating Quasar Parameters

Yet to implement-

Byzantine condition and IFE-Claims
ERC-20 support
Anti front-running and service provider fee

souradeep-das · 2020-12-02T09:23:34Z

Funds flow in/out the contract

TokenCapacity

--> addEthCapacity() increases capacity
----> obtainTicket() decreases capacity
------> flushExpiredTicket() can increase the capacity back
------> challengeClaim() can increase the capacity back
else, withdrawal successful, capacity decreased

Bond

--> obtainTicket() adds bond value to contract
----> flushExpiredTicket() adds bond to bond reserve (stays in the contract)
----> challengeClaim() returns bond to challenger
----> processClaim() returns bond to output owner

bondReserve is collective amount of bonds that is not blocked by any claim. can be withdrawn

souradeep-das · 2020-12-02T11:14:18Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+     * @notice Only an unclaimed ticket can be flushed, bond amount is added to bondReserve
+     * @param utxoPos pos of the output, which is the ticket identifier
+    */
+    function flushExpiredTicket(uint256 utxoPos) public {


Alternatively, we can push tickets to queue, to flush expired tickets together according to timestamp, but guessing there might not be many expired unclaimed tickets because of the bond

souradeep-das · 2020-12-02T11:15:47Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+     * @dev Process the Claim to get liquid funds
+     * @param utxoPos pos of the output, which is the ticket identifier
+    */
+    function processClaim(uint256 utxoPos) public {


Alternatively, we can maintain a queue for processing multiple claims together

souradeep-das

Also, there is a scope to remove utxoPos from the args required for claim() and challengeClaim(). I have kept it for now for simplicity and readability but this might increase gas.
Any suggestions about these design decisions?

InoMurko

1337

InoMurko

I noticed I didn't look at the correct PR.

InoMurko · 2020-12-29T19:35:31Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        tokenUsableCapacity[ticketData[utxoPos].token] = tokenUsableCapacity[ticketData[utxoPos].token].add(tokenAmount);
+        bondReserve = bondReserve.add(ticketData[utxoPos].bondValue); 


Suggested change

tokenUsableCapacity[ticketData[utxoPos].token] = tokenUsableCapacity[ticketData[utxoPos].token].add(tokenAmount);

bondReserve = bondReserve.add(ticketData[utxoPos].bondValue);

tokenUsableCapacity[ticketData[utxoPos].token] = tokenUsableCapacity[ticketData[utxoPos].token].add(tokenAmount);

bondReserve = bondReserve.add(ticketData[utxoPos].bondValue);

this is basically returning the reserved amount back to the Quasar and claim the bond?

Yes! It's just adding to the amount of bonds from expired tickets, that the quasar owner can withdraw.(failed bonds)
I'll redirect you to this for the flow #723 (comment)

plasma_framework/contracts/poc/fast_exits/Quasar.sol

InoMurko · 2020-12-29T19:42:20Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+            tokenUsableCapacity[token] = tokenUsableCapacity[token].sub(residualAlmount);
+            emit QuasarTotalEthCapacityUpdated(tokenUsableCapacity[address(0x0)]);
+        }
+        msg.sender.transfer(amount);


does this not apply here? https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

yeah, I should change this. Thanks for catching!

InoMurko · 2020-12-29T19:47:27Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+     * @param rlpOutputCreationTx RLP-encoded transaction that created the output
+     * @param outputCreationTxInclusionProof Transaction inclusion proof
+    */
+    function obtainTicket(uint256 utxoPos, bytes memory rlpOutputCreationTx, bytes memory outputCreationTxInclusionProof) public payable {


I noticed we sometimes use underscores to differentiate global params and function arguments, is this a thing we should be doing?

Yeah, I think what we followed in the past is to move away from underscore prefixes unless there is a var name collision, probably because of possible confusion of it being an unused var 😄

InoMurko · 2020-12-29T19:48:37Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        require(msg.value == bondValue, "Bond Value incorrect");
+
+        tokenUsableCapacity[outputData.token] = tokenUsableCapacity[outputData.token].sub(outputData.amount);
+        ticketData[utxoPos] = Ticket(msg.sender, block.timestamp.add(14400), outputData.amount, outputData.token, msg.value, rlpOutputCreationTx, false);


would an emmited event be useful here? I'm not sure.

Sure, we definitely need more events.

InoMurko · 2020-12-29T20:17:12Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        require(MoreVpFinalization.isStandardFinalized(
+            plasmaFramework,
+            rlpTxToQuasarOwner,
+            utxoQuasarOwnerDecoded.toStrictTxPos(),
+            txToQuasarOwnerInclusionProof
+        ), "Provided Tx doesn't exist");


If I understand this correctly, the output needs to be transfered to the quasar owner on the actuall plasma chain and it has to be included in the block, correct?

This means, from the UX perspective:

initate an exit on plasma framework contracts

get ticket from quasar for the exit

after the ticket from the quasar, use the same data to make a transfer via Watcher to ChildChain and transfer that output to the quasar owner and wait for a block to be submitted

now claim

is that correct?

is there a way to merge any of these steps?

perhaps:
quasar emits an event that marks the output from alice that wants to exit as spent
and quasar emits an event that "makes a fake deposit" that basically transfers the ownership of the output to the quasar owner?

maybe that's actually one event emmited. this events would be tracked by the childchain and watcher.

Yes correct, for the standard way of doing the claim, you have to spend the output to the quasar owner and it has to be included in a block, (there is IFEClaim for tx not included, which is more of a way to recover funds)
The initiate exit on plasma is not required, instead just spending the output
so the UX will be -

get ticket from quasar with an output

after the ticket from the quasar, make a transfer via Watcher to ChildChain and spend that same output to the quasar owner and wait for a block to be submitted

Now Claim (there is no processClaim if there is no waiting period on quasar, so they can be merged)

Merging some steps on the UX would be interesting to look at. I think looking at it from the core, the 1st step is contract, 2nd is Plasma and 3rd is contract again. I was thinking if 1 and 2 could be one step, doing the second on a success receipt of 1, but still would require two signing?

yeah, these are good thoughts to explore 👍

is it safe to merge step 1 and 2 by emitting an event? so that the childchain and watcher spend the output and there's no need to manually send a transaction from alice to quasar owner?

umm this is tricky, currently the watcher would expect the tx to be correclty signed by alice after the block is submitted and also if there is a need to start exit on this tx later, it won't happen if it isn't signed by alice, so I think merging the steps from a ux perspective should be good as long as we get alice to sign both times? I might be wrong though

for 3, I feel it should be fine for anyone to call the claim func with the parameters. so maybe maybe there is a chance here that we could have a bot automate 3 after 2?

yes absolutely

on the topic of 1 and 2.
what if we make the transaction between alice and quasar - like a deposit event?
and alice, since this is coming from her, could still combine an output spending into the deposit?

okay... I might be missing something. Brainstorming here:

ticket claiming emits an event that combines an output spenditure + a deposit event to the quasar owner?

interesting! one thing i think will stand in the way is - the need of alice's output to be an input in the tx to the quasar owner in order to verify the claim. in case we mark the output as spent and send a deposit event, we won't have inputs on the deposit tx, and still thinking if there is a way to verify alice spent her output. but worth exploring.
Also to mark alice's output as spent we need a tx that spends that output? since this tx will be used to challenge alice exiting that output. So we would still need a signed tx from alice anyway?

InoMurko · 2020-12-30T13:41:13Z

any particular obstacles if we merge flushExpiredTicket (by iterating through the list of tickets and checking block.timestamp > expiryTimestamp && expiryTimestamp != 0)?

souradeep-das · 2020-12-30T13:50:57Z

Oh do you mean to collectively flush all expired tickets? we can totally do that

InoMurko · 2020-12-30T13:53:19Z

Oh do you mean to collectively flush all expired tickets? we can totally do that

Yeah, but also, maybe we can do that while someone calls obtainTicket/3? Or would that be too expensive?

souradeep-das · 2020-12-30T13:59:28Z

ohh so attempting to flush expired tickets from the list everytime a ticket obtained? that's a possibility. Yeah that might be expensive, and we might expect very less tickets to go expired beacuse of the bond ( we should call it unclaimed instead of expired ticket) only the tickets that weren't claimed will need to be flushed

kevsul

In general looks good.
I've updated many of the error messages for readability, but feel free to modify my suggestions.

plasma_framework/contracts/poc/fast_exits/Quasar.sol

kevsul · 2021-01-04T15:19:08Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+
+        require(outputData.amount <= tokenUsableCapacity[outputData.token], "Requested amount exceeds the Usable Liqudity");
+        require(outputData.amount != 0, "The reserved amount cannot be zero");
+        require(msg.value == bondValue, "Bond Value incorrect");


Can we rearrange these checks to fail sooner when possible?
This one could be the first check, for example

plasma_framework/contracts/poc/fast_exits/Quasar.sol

kevsul · 2021-01-04T16:00:53Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+     * @param utxoPos pos of the output, which is the ticket identifier
+    */
+    function verifyTicketValidityForClaim(uint256 utxoPos) private {
+        require(!ticketData[utxoPos].isClaimed, "Already Claimed");


Suggested change

require(!ticketData[utxoPos].isClaimed, "Already Claimed");

require(!ticketData[utxoPos].isClaimed, "Already claimed");

kevsul · 2021-01-04T16:01:48Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        = PaymentTransactionModel.getOutput(decodedTx, 0);
+
+        // verify output to Quasar Owner
+        verifyOwnership(outputData, quasarOwner);


The error message if this fails doesn't make sense - see comment on verifyOwnership() above

kevsul · 2021-01-04T16:02:12Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        // verify output to Quasar Owner
+        verifyOwnership(outputData, quasarOwner);
+        // considering fee as a separate input
+        require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong Amount Sent to Quasar Owner");


Suggested change

require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong Amount Sent to Quasar Owner");

require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong amount sent to quasar owner");

kevsul · 2021-01-04T16:02:30Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        verifyOwnership(outputData, quasarOwner);
+        // considering fee as a separate input
+        require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong Amount Sent to Quasar Owner");
+        require(ticketData[utxoPos].token == outputData.token, "Wrong Token Sent to Quasar Owner");


Suggested change

require(ticketData[utxoPos].token == outputData.token, "Wrong Token Sent to Quasar Owner");

require(ticketData[utxoPos].token == outputData.token, "Wrong token sent to quasar owner");

kevsul · 2021-01-04T16:04:00Z

plasma_framework/contracts/poc/fast_exits/Quasar.sol

+        = PaymentTransactionModel.decode(claimTx);
+
+        // first input should be the Utxo with which ticket was obtained
+        require(decodedTx.inputs[0] == bytes32(utxoPos), "Wrong Tx provided");


Suggested change

require(decodedTx.inputs[0] == bytes32(utxoPos), "Wrong Tx provided");

require(decodedTx.inputs[0] == bytes32(utxoPos), "The claim transaction does not spend the correct output");

Co-authored-by: Kevin Sullivan <4653170+kevsul@users.noreply.github.com>

feat: implement happy withdrawal path

29ad41f

souradeep-das added 2 commits December 2, 2020 15:02

style: sol lint

e3e4287

style: sol lint

e318faf

souradeep-das commented Dec 2, 2020

View reviewed changes

souradeep-das requested review from boolafish, kevsul, madxor and thec00n December 2, 2020 11:27

InoMurko reviewed Dec 3, 2020

View reviewed changes

souradeep-das added 4 commits December 3, 2020 17:46

refactor: use safemath wherever unused

9453af7

fix: minor warnings

c557e30

fix: minor warnings

6a30e0f

refactor: challengeClaim to use same method for ifeClaim challenges

e042206

boolafish removed request for thec00n and boolafish December 22, 2020 09:01

InoMurko reviewed Dec 29, 2020

View reviewed changes

souradeep-das mentioned this pull request Jan 4, 2021

Fast Exit Construction #726

Open

fix: safe transfer

42e8889

kevsul reviewed Jan 4, 2021

View reviewed changes

souradeep-das and others added 2 commits January 5, 2021 17:04

style: changes from review

3457816

Update plasma_framework/contracts/poc/fast_exits/Quasar.sol

11ec1d7

Co-authored-by: Kevin Sullivan <4653170+kevsul@users.noreply.github.com>

souradeep-das requested review from kevsul and InoMurko and removed request for madxor January 11, 2021 07:08

InoMurko approved these changes Jan 12, 2021

View reviewed changes

kevsul approved these changes Jan 12, 2021

View reviewed changes

souradeep-das merged commit bf6c44c into v2.0.0 Jan 14, 2021

souradeep-das deleted the souradeep/quasar branch January 14, 2021 08:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quasar Fast Exit: Implement withdrawal the happy path #723

Quasar Fast Exit: Implement withdrawal the happy path #723

souradeep-das commented Dec 2, 2020 •

edited

souradeep-das commented Dec 2, 2020

souradeep-das Dec 2, 2020

souradeep-das Dec 2, 2020

souradeep-das left a comment •

edited

InoMurko left a comment

InoMurko left a comment

InoMurko Dec 29, 2020

souradeep-das Dec 30, 2020

InoMurko Dec 29, 2020

souradeep-das Dec 30, 2020

InoMurko Dec 29, 2020

souradeep-das Dec 30, 2020

InoMurko Dec 29, 2020

souradeep-das Dec 30, 2020

InoMurko Dec 29, 2020

souradeep-das Dec 30, 2020

InoMurko Dec 30, 2020

souradeep-das Dec 30, 2020 •

edited

souradeep-das Dec 30, 2020

InoMurko Dec 30, 2020

InoMurko Dec 30, 2020 •

edited

souradeep-das Dec 30, 2020

InoMurko commented Dec 30, 2020

souradeep-das commented Dec 30, 2020

InoMurko commented Dec 30, 2020

souradeep-das commented Dec 30, 2020

kevsul left a comment

kevsul Jan 4, 2021

kevsul Jan 4, 2021

kevsul Jan 4, 2021

kevsul Jan 4, 2021

kevsul Jan 4, 2021

kevsul Jan 4, 2021

		tokenUsableCapacity[ticketData[utxoPos].token] = tokenUsableCapacity[ticketData[utxoPos].token].add(tokenAmount);
		bondReserve = bondReserve.add(ticketData[utxoPos].bondValue);

	require(!ticketData[utxoPos].isClaimed, "Already Claimed");
	require(!ticketData[utxoPos].isClaimed, "Already claimed");

	require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong Amount Sent to Quasar Owner");
	require(ticketData[utxoPos].reservedAmount == outputData.amount, "Wrong amount sent to quasar owner");

	require(ticketData[utxoPos].token == outputData.token, "Wrong Token Sent to Quasar Owner");
	require(ticketData[utxoPos].token == outputData.token, "Wrong token sent to quasar owner");

	require(decodedTx.inputs[0] == bytes32(utxoPos), "Wrong Tx provided");
	require(decodedTx.inputs[0] == bytes32(utxoPos), "The claim transaction does not spend the correct output");

Quasar Fast Exit: Implement withdrawal the happy path #723

Quasar Fast Exit: Implement withdrawal the happy path #723

Conversation

souradeep-das commented Dec 2, 2020 • edited

Overview

souradeep-das commented Dec 2, 2020

Funds flow in/out the contract

TokenCapacity

Bond

Choose a reason for hiding this comment

Choose a reason for hiding this comment

souradeep-das left a comment • edited

Choose a reason for hiding this comment

InoMurko left a comment

Choose a reason for hiding this comment

InoMurko 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

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

souradeep-das Dec 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InoMurko Dec 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InoMurko commented Dec 30, 2020

souradeep-das commented Dec 30, 2020

InoMurko commented Dec 30, 2020

souradeep-das commented Dec 30, 2020

kevsul 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

souradeep-das commented Dec 2, 2020 •

edited

souradeep-das left a comment •

edited

souradeep-das Dec 30, 2020 •

edited

InoMurko Dec 30, 2020 •

edited