Funding fraud refactor #534
Conversation
Funding Fraud now completes on provideFundingECDSAFraudProof and always costs the signers their full bond as long as the funding timeout has not elapsed
temove tests for notifyFraudFundingTimeout and provideFraudBTCFundingProof
partiallySlashForFraudInFunding was only called by notifyFraudFundingTimeout, which was removed
If the funding proof timeout has elpased, the funder does not recieve the signer bond. It is burned instead.
Test that the signer bond is always seized and the funder is awarded the bond if the timeout has not elapsed.
Bond can be a const assigned directly
| /* NB: This is reuse of the variable */ | ||
| _d.fundingProofTimerStart = block.timestamp; | ||
| _d.setFraudAwaitingBTCFundingProof(); | ||
| distributeSignerBondsToFunder(_d); |
There was a problem hiding this comment.
If we still distinguish between funding timeouts, we allow the signers to grief the funder by observing a late funding tx, committing fraud, and then losing their bond. The funder is still out their BTC value completely, though the system is in balance and the signers have lost +50% value (more or less).
If we remove the distinction, we allow the funder to gain the ETH bond even if they haven't funded the deposit, but only if they convince the funders to collude. So they can get +150% value (more or less), but it requires them convincing signers to actively do the wrong thing, rather than simply make a mistake. We also simplify the code. I'm tempted to take this path…
Thoughts?
There was a problem hiding this comment.
Thoughts?
Agree. As a note:
f we remove the distinction, we allow the funder to gain the ETH bond even if they haven't funded the deposit, but only if they convince the funders to collude.
This shouldn't be a concern, making the change even better
provideFundingECDSAFraudProof should always transfer signer bond to funder. This avoids some signer griefing vectors while simplifying the logic
provideFundingECDSAFraudProof time disctinction was removed.
|
just dropping a quick summary of changes here: removes state
changes:
|
It does! Keep closing is now part of |
| _d.logSetupFailed(); | ||
|
|
||
| partiallySlashForFraudInFunding(_d); | ||
| distributeSignerBondsToFunder(_d); |
There was a problem hiding this comment.
Note: funder can cause this to fail and potentially push a fraudulent deposit into an active state.
There was a problem hiding this comment.
Indeed, this feeds into the overall “push vs pull” question noted in a few other places as well:
- Block liquidationInitiator fallback revert on purchaseSignerBondsAtAuction #524 (comment) (liquidation)
- Refund funder if signer setup fails #520 (comment) (also funding, but a different scenario)
- I believe a couple of other places we haven't called out explicitly but that we're taking stock of at the moment.
refs: #494
Members of the signing group might decide to call notifyFraudFundingTimeout in a race to avoid late submissions for provideFraudBTCFundingProof to succeed in order to contain funds lost due to fraud.
provideFundingECDSAFraudProofwith the signersalways losing their full bind as long as the funding timeout has not elapsed.