-
Notifications
You must be signed in to change notification settings - Fork 40
protocol: Spam deterrence #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: hardfork/v4
Are you sure you want to change the base?
Conversation
This issue has been assigned a bounty (💰)Current bounty:
Exceptions for this specific bountiesDue to the nature of this issue (cryptographic/protocol complexity rather than implementation complexity) the bounty will also be used to fund audits. Bounty donation addressIf you want to incentivize work on this issue, you can help increase the bounty by donating to the address below. Fine printWe use bounties to incentivize development and reward contributors. All issues available for a bounty have the To receive the bounty of this issue, you agree to these conditions:
|
|
Can you elaborate when partial refund will be done instead of full refund and punish? It seems like Bob needs Alice to get 10% of his money back in case of partial refund. |
|
Yes. This update will essentially move some trust requirement from the maker (that the taker is not a spammer) to the taker (that the maker will refund). However, this can be addressed as follows:
Everyone would be better off if this wasn't necessary, but it is. That's the sad truth. |
|
I am not sure if i and others understand your words "takers might choose to only start swaps for which the maker has already provided the full refund address. Thus not requiring the maker to fully refund" can you rephrase it please? sorry "the maker does not gain anything from withholding the refund, thereby he is not incentivized to do so" Can you elaborate when partial refund will be done instead of full refund and punish? |
I am also trying to understand but here is how I interpreted it: Makers will have the choice to offer full refunds or partial refunds. The taker will know the maker's refund policy before they begin a swap. The full refund policy is straightforward: If the taker cancels, they get the full refund. For the partial refund policy, if the taker cancels, they are not guaranteed to receive 100% of their money back. The purpose of this is to disincentivize malicious takers from abandoning swaps because they know there is no penalty for doing so. If you as a taker do not like the idea of a partial refund, you as a free market participant, can refuse to do business with that maker. Because partial refunds mean less risk to the maker, this will allow them to offer lower rates.
He lost his money due to negligence (or at least failure to understand the refund process). As the docs state "With most makers, you can still redeem the Monero even after being punished. This is, however, purely voluntary and we advise against relying on this." |
|
That would mean either makers are ok with full refund = they automatically give it, or they are not ok with full refund, meaning they will also not help giving it. Its just 10% scam. Its not even remotely justified as fees of all makers are currently around 2%, why should a taker loose 10%? |
It's not a scam if you know what you are agreeing to. The taker also doesn't lose anything if they simply complete the swap like they agreed to. Makers are not obligated to hand out free 12 hour XMR call options. Adding a partial refund support will actually help honest takers. If there is not penalty, a maker can lock up the liquidity of other makers that offer lower rates than them. Then they can jack up their rate because their competition is gone. This is effectively a Denial of Service attack against makers. |
|
If thats the worry the penality fee should at maximum be the fee that they ask from the user for a completed trade, not a flat 10% |
The XMR price movement also needs to be taken into account here — it can easily move 5–8% during those 12 hours. |
|
It is not a flat 10%. The 10% is purely for demonstrational purposes. The percentage can be dynamically configured by the maker. |
I wanted to specify this in a bit more in detail as they seems to be some confusion around the bounty on this issue. In general donations to specific issues will NEVER go directly to neither me nor @Einliterflasche. Issue specific bounties are for funding work of outside contributors. Donations to this issue will not necessarily result in me implementing this in a faster manner. Donations will be used to potentially fund an outside contributor to give this a review to assess the security of this on the protocol level. As the funds collected here are unlikely to be enough to fund a full audit (something like TrailOfBits would do) this is going to be "light review" most likely with no attribution to the reviewer. We are in talks with someone who is well trusted in the Monero community and is more than enough qualified. If we don't find a reviewer, the funds will be used for other bounties. |
|
Shouldnt a protocol change be fully audited? |
No, not at all protocol changes necessarily require an audit. We are not introducing any new novel cryptography here. I understand the concern here though. This will obviously be well tested. |
src-gui/src/renderer/components/alert/SwapStatusAlert/SwapStatusAlert.tsx
Show resolved
Hide resolved
266e3a1 to
c5e0871
Compare
c5e0871 to
f46f4ee
Compare
…fund.rs and add backwards compatible BobRefundType for abstracting over refund signatures
| // We sent Bob the encsig for the full refund path already, so we don't | ||
| // care about the partial refund path signatures of Bob anyway. | ||
| // We just save `None`. | ||
| if self.btc_amnesty_amount.unwrap_or(bitcoin::Amount::ZERO) == bitcoin::Amount::ZERO { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here as above, also perhaps extract this into some trait that allows checking if the amount is high enough to construct a transaction for it?
Currently there is no reputation system. But now that makers can, in theory, burn some of the refund of the takers we still want to incentivize not doing that. The taker could proof that the maker burnt the funds by sharing TxRefundBurn. This means takers can avoid makers who are shown to haven often burnt refunds in the past. |
| pub mod containers; | ||
| pub mod images; | ||
|
|
||
| use anyhow as _; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this necessary? maybe we should hide some of these crates behind a flag if there only used for the binary?
swap-p2p/src/out_event/alice.rs
Outdated
| // should go into the amnesty output | ||
| send_wallet_snapshot: bmrng::RequestReceiver< | ||
| bitcoin::Amount, | ||
| (swap_setup::alice::WalletSnapshot, bitcoin::Amount), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract this into a named tuple or add an explicit comment above this line (// (snapshot, amnesty_amount)
| #[typeshare] | ||
| pub struct BidQuote { | ||
| /// The price at which the maker is willing to buy at. | ||
| #[serde(with = "::bitcoin::amount::serde::as_sat")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its good to keep these for completeness snake. We want the network protocols serialization to be defined as explicitly as possible.
| pub enum SpotPriceError { | ||
| NoSwapsAccepted, | ||
| AmountBelowMinimum { | ||
| #[serde(with = "::bitcoin::amount::serde::as_sat")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, I think we should keep them
| /// Bob locks Btc and Alice locks Xmr. Alice does not act so Bob does a partial | ||
| /// refund, waits for the remaining refund timelock, and then claims the amnesty. | ||
| #[tokio::test] | ||
| async fn given_partial_refund_bob_claims_amnesty_after_timelock() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need another integration tests for the other paths
while the idea is great its easily avoided: just swipe setup and get new maker identity if labeled as described above |
I think there is an important asymmetry here: maker identities are much more costly to rotate than taker identities. A taker can reset their identity with a simple reinstall, while a maker typically runs long-lived infrastructure with stable liquidity and uptime. |
|
As a side note, the previously used “went online X days ago” metric (or similar) could also be a useful additional signal for makers, as it naturally reflects identity persistence and operational stability. |
|
Maintaining a simple taker peer ID–based blacklist can be quite effective, too |
I noticed the spammer (linked to the same btc), usually rotates peer ids now |
…esty{Granted, Published, Confirmed}
|
Bumping this from a draft PR to normal PR in order to run the tests. TODO:
|
|
Integration tests for full refund, partial refund + amnesty are passing in the stagenet environment. Also TODO: rename the transactions/states to something more sensical |
|
The tx_burn_refund transaction is modified such that Alice needs to leak her private key (used for just this one swap) when burning the refund. The same goes for the Bitcoin punish transaction. Additionally, Bob will transmit the encrypted signature to Alice by sending a Monero transaction to the shared Monero wallet where the encrypted signature is included in the tx_extra field of the transaction ( This will allow Alice and Bob to construct a proof which can be shared with others. When verified a third party (Carol) can check whether:
This allows Carol to "blame" one of the parties for the failed swap and she can make decision not to trust the misbehaved party. Alice might share the proof to warn others of Bobs behavior. Other makers might blacklist Bob after verifying the proof. Bob might share the proof if Alice misbehaved. Other takers might decide not to swap with Alice anymore. This essentially allows for a reputation system which is purely focused on negative examples. A proof consists of:
Additionally, the signature is signed by both Bob and Alice using their Peer ID. The full public key is also included for each peer. Both Alice and Bob "commit" to the proof. Bob can also append the Monero transaction hash of the transaction that reveals the encrypted signature to Alice. Alice can append the Monero transaction hash of the transaction that funds the shared Monero wallet. These are included to make proof verification faster. During verification Carol will do the following (for each of the cases):
note: this needs to be written down in a cleaner spec and will be implemented in another PR (different from this one) |
This PR introduces a change in the atomic swap protocol to discourage spam. It does so by attaching a fee to refunds. This fee will be burnt.
Current situation
The exchange rate of each swap is set at the start and can't be changed.
A swap either completes within 12 hours (this requires cooperation by both parties), or be cancelled.
A swap might still be cancelled after both parties lock their respective currency.
In that case the taker ("Bob") can issue a refund within a 24 hours window. He will get his Bitcoin back and the maker ("Alice") will get her Monero back.
The problem
This can be abused by malicious takers in two ways:
This is possible by initiating a swap and waiting for the maker to lock the Monero.
Then not continue the swap and wait for a refund.
This will only cost the malicious actor transaction fees for 3 Bitcoin transactions, while preventing the "small maker" from competing in the market for trading volume.
If that doesn't happen they can refund for the low cost of 3 Bitcoin transactions.
This allows the malicious actor to treat the swap like a (bascially) free Monero call option.
The solution
The root problem causing both these issues is that refunds are free (for the taker).
By attaching a cost to refunds (only refunding a part of the Bitcoin) this is no longer the case.
Thus both tactics are no longer profitable.
Makers will be able to set the extend of the refund fee. Takers will know the refund fee prior to starting a swap and decide whether to trade or not.
The refund fee does not go to the maker. That would create skewed incentives. The refund fee will be "burnt". The only way to retrieve it is for the maker to sign a transaction giving it back to the taker.
However, the maker will be able to voluntarily give the taker a full refund. This is what we call "amnesty" for now. This is intended such that makers can help out clueless takers who accidentally refunded.
This path is, however, purely voluntary and takers can't 100% rely on it.
Implementation
TxPartialRefundTxRefundAmnestyamnesty_amount,tx_partial_refund_fee,tx_refund_amnesty_fee,tx_partial_refund_encsigand optionallytx_full_refund_encsigandtx_refund_amnesty_sigTxFullRefundandTxPartialRefund