eth/contract_V0 [discussion]: Remove msg.sender == * checks.

[JoeGruffins]

Currently the message sender is checked to be one of the initiator or the participant for init, redeem, and refund. I propose we remove these checks for a few reasons:

1. The main benefit is that this would allow anyone to redeem or refund a swap. As you know, it takes gas to work with a contract. A user could get stuck in a position where they cannot pay the fee to do one of these. This can be a time sensitive operation for the taker, who may need to redeem or refund before the maker's locktime is passed to avoid losing funds. Having no requirement on the sender would allow a "good Samaritan" to help the user out, however unlikely that may be.

2. It may reduce gas usage by a small amount for initiations and refunds. For redeems, if we want to send all the funds at once I guess there will still need to be a check that all participants are the same address.

3. It is not needed. We choose the initiator/refunder and participant/redeemer at initiation. As long as anyone knows the secret, we can make sure they can only redeem to the participant. If locktime is past, we can ensure the refund only goes to the initiator. I believe we check now because that's what UTXO coins do. They have the limitation that one cannot choose who a utxo is spent to when constructing a script to be signed. We ensure it can only be signed by the owner of a certain private key, and assume the owner will spend to themselves. But with a contract we can specify that the locked value is only paid to a specific party.

Of course please let me know if I overlooked another obvious reason for the checks, or if point 3 is not true.

[buck54321]

This makes sense for initiation. The restrictions are unnecessarily restrictive to 1) third-party services and 2) privacy-focused clients. Like you said, the initiator and participant are set in the initiation. By design, only these parties can receive funds from refund or redeem. That seems to cover our end as far as responsibilities go.

For refunds and redeems, this carries additional complexity, since anyone with the secret, including the counter-party (if taker) and the server, could redeem for us. For refunds, anyone could then refund for us after the lock time. Neither results in a loss of funds, and both are acceptable outcomes from an accounting perspective. But we would have to program around those possibilities.

[martonp]

I agree, the checks are pointless and waste gas. I guess we could add a "good samaritan" script to allow a user to help someone else redeem.

[chappjc]

For refunds and redeems, this carries additional complexity, since anyone with the secret, including the counter-party (if taker) and the server, could redeem for us. For refunds, anyone could then refund for us after the lock time. Neither results in a loss of funds, and both are acceptable outcomes from an accounting perspective. But we would have to program around those possibilities

This is the part that might be trouble for clients. The Core logic is going to get angry if it finds the refund or redeem is already done. We can almost certainly code for these possibilities, but it's something we will have to deal with and I don't know how much machinery we'll need to add to work with this. Hopefully it will be as simple as adding an ErrAlreadyRedeemed error (e.g.) in the low level node client that gets recognized in the higher level method Wallet.Redeem, which then returns a nil error and the valid Coin back to Core.

Worth a shot, but we cannot delay much on this as we have a Solidity audit to secure.

[JoeGruffins]

Will do this.

[JoeGruffins]

Actually, init does not check if sender is the initiator, it just sets the sender to initiator. So, in order to accept any initiator, the argument struct needs another parameter. Also, the redeem check that we send to the same address seems to take a bit of gas.

Will probably look like:

var v0Gases = &dex.Gases{
        Swap:      135000 -> 135200,
        SwapAdd:   113000 -> 113200,
        Redeem:    63000 -> 65000,
        RedeemAdd: 32000 -> 34000,
        Refund:    43000 -> 41000,
 }

So from a fee standpoint, not really worth it. Using msg.sender is more convenient. Refund is less, but we would expect that to be used the least.

We could just remove the refund barrier as well.

If anyone has an opinion knowing the new gas prices.

[martonp]

I think in that case I don't think there's really a point. Especially if it requires complicated changes to the client.

The "good samaritan" could just send the redeemer/refunder some funds instead of redeeming in their place and there would only be a few minute delay. Creating the service where a third party redeems in place of a user for a fee would require a mechanism for the third party to get part of the redemption.. this change would not be enough.

[chappjc]

Yeah, doesn't seem like there is any point. Would have to be a good gas savings IMO to warrant the change. oh well.

But I'm thinking back to the "backup refund" goal #1311, and wondering if we can call that solved if we can verify (on Goerli) that the ETH seed from assetseed can be used with MetaMask/MEW or another web3 wallet following the bip44 derivation path, because it I believe it would be dead simple to manually refund with say etherscan and Connect to Web3:

That would open the connected web3 wallet to prep a txn to do the refund.
It's just unclear how various wallets define their "Secret Recovery Phrase" w.r.t. seed bytes.

[JoeGruffins]

Again, we could just remove the check for refunds, which would make it slightly less restrictive, and then anyone could refund from any wallet with just the secret hash, which I guess could solve #1311 without a seed.

Plus less gas there.