This repository has been archived by the owner on Aug 2, 2021. It is now read-only.
swap: refactor cashout logic to cashout processor #2066
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
Eknir
reviewed
Jan 8, 2020
contracts/swap/swap.go
Outdated
| @@ -48,8 +49,10 @@ type Contract interface { | |||
| Withdraw(auth *bind.TransactOpts, amount *big.Int) (*types.Receipt, error) | |||
| // Deposit sends a raw transaction to the chequebook, triggering the fallback—depositing amount | |||
| Deposit(auth *bind.TransactOpts, amout *big.Int) (*types.Receipt, error) | |||
| // CashChequeBeneficiary cashes the cheque by the beneficiary | |||
| CashChequeBeneficiary(auth *bind.TransactOpts, beneficiary common.Address, cumulativePayout *big.Int, ownerSig []byte) (*CashChequeResult, *types.Receipt, error) | |||
| // CashChequeBeneficiaryStart sends the transaction to a cash a cheque as the beneficiary | |||
There was a problem hiding this comment.
Small typo here: "to a cash a cheque"
There was a problem hiding this comment.
It still says: to a cash a cheque. Removing the first a would fix it
There was a problem hiding this comment.
fixed now. the same comment existed in two places and I only had fixed one.
Eknir
reviewed
Jan 8, 2020
holisticode
reviewed
Jan 9, 2020
holisticode
reviewed
Jan 9, 2020
holisticode
reviewed
Jan 9, 2020
swap/cashout_test.go
Outdated
| } | ||
|
|
||
| // the gas price in the simulated backend is 1 therefore the total transactionCost should be 50000 * 1 = 50000 | ||
| if transactionCost != 50000 { |
There was a problem hiding this comment.
should we extract this into a constant?
holisticode
approved these changes
Jan 9, 2020
Eknir
reviewed
Jan 10, 2020
contracts/swap/swap.go
Outdated
| @@ -48,8 +49,10 @@ type Contract interface { | |||
| Withdraw(auth *bind.TransactOpts, amount *big.Int) (*types.Receipt, error) | |||
| // Deposit sends a raw transaction to the chequebook, triggering the fallback—depositing amount | |||
| Deposit(auth *bind.TransactOpts, amout *big.Int) (*types.Receipt, error) | |||
| // CashChequeBeneficiary cashes the cheque by the beneficiary | |||
| CashChequeBeneficiary(auth *bind.TransactOpts, beneficiary common.Address, cumulativePayout *big.Int, ownerSig []byte) (*CashChequeResult, *types.Receipt, error) | |||
| // CashChequeBeneficiaryStart sends the transaction to a cash a cheque as the beneficiary | |||
There was a problem hiding this comment.
It still says: to a cash a cheque. Removing the first a would fix it
…estDeployWithPrivateKey
ralph-pichler
force-pushed
the
swap_cashout_3
branch
from
b51d350
to
a148161
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 PR moves most of the cashout related logic into its own file (
cashout.go) and onto its own receiver (CashoutProcessor) and splitsCashChequeBeneficiary(in thecontractpackage) intoCashChequeBeneficiaryStartandCashChequeBeneficiaryResult.This by itself does not change any functionality. The changes made here are primarily to make implementing a proper cashout queue easier (as the PR for that was getting out of hand in terms of complexity). The
CashoutProcessorwas introduced as the responsibilities of theSwapstruct are already huge and cashing is orthogonal to the rest of the swap logic.This PR introduces several structs and passes them around instead of individual parameters. The reasoning behind this is that in a later PR this structs will be persisted to the local store to allow queueing wand waiting fo cashouts across client restarts.
In addition this PR also:
estimatePayoutfunctionDefaultTransactionTimeout)testDeployWithPrivateKeyto make tests which don't need the full swap easiernewSignedTestChequeto make creating signed cheques for tests easier