atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard

feat(utxo-swap): add utxo burn output for non-kmd taker fee

Open dimxy opened this issue 1 year ago • 1 comments

Adds a burn output sending 25% of the taker utxo DEX fee to a dedicated pre-burn address. Funds collected on the pre-burn address will be traded for KMD to burn them (thus additionally burning KMD supply). This PR partially closes https://github.com/KomodoPlatform/komodo-defi-framework/issues/2010

In this PR:

  • [x] split dex fee for non-kmd utxo coins (apart from zcoin) and added an output to the pre-burn account
  • [ ] old-style non-split dex fee non-kmd txns are also allowed in validation code (until all taker nodes upgraded) - this feature is removed in favour of version-based burning activation
  • [x] refactored dex fee pubkey in params: instead of passing dex fee and burn pubkeys in function params new methods dex_pubkey() and burn_pubkey() were added to the SwapOps trait
  • [x] add version to maker/taker negotiation message and activate burning for the new version
  • [x] mocktopus was made optional dependency and activated only for development builds (as its doc suggests).

NOTE: As mocktopus now is marked 'optional = true' in coins Cargo.toml and activated from the mm2_main crate by adding features = ["mocktopus"] in [dev-dependencies] section, you also need to mark your mockable code, called from other crates, this way: #[cfg_attr(feature = "mocktopus", mockable)], otherwise mocks won't work (see samples in code)

TODO:

  • [ ] fix burn pubkey and burn zaddr
  • [x] Add private burn output for zcoin
  • [ ] disable non-split non-kmd dex fee (no burn output) validation when all taker nodes upgrade to new dex fee splitting
  • [x] do not pay dex fee if taker is the dex pubkey (for non-privacy utxo)

dimxy avatar May 06 '24 18:05 dimxy

In the recent 16 commits I added a 'version' field to negotiation protocol. This is done in the current PR to activate burning of non-kmd part of dex fee (a new tx output to the pre-burn account) by node's supported protocol version. However I guess version is a useful feature for other future protocol changes (so this PR closes this issue #640 too).

How negotiation with version is working, also to provide compatibility with old nodes (see MakerSwap::negotiate and TakerSwap::negotiate fn):

Now new Maker sends both versioned and old non-versioned negotiation message to the Taker. If Taker is new:

  • New Taker receives the versioned message first and responses with the versioned negotiation reply (and ignores the old negotiation message). I estimate this ordering (versioned message is sent first, non-versioned is second) should work.
  • New Maker receives the versioned reply and both new Taker and new Maker now know version of each other (now it is 1) and use the new DexFee (with the burning output).

If Taker is old:

  • it ignores the versioned message and responses with old reply.
  • New Maker receives the old reply and now knows the Taker is old (its version is 0) - both use old DexFee Also, if Maker is old and Taker is new - new Taker would respond with old non-versioned reply and so on.

When all nodes upgrade we can remove old non-versioned negotiation protocol support.

I added two cargo features to allow to emulate old node behaviour (sending always non-versioned negotiation request or reply) for use with docker tests, like test_trade_base_rel_mycoin_mycoin1_coins

I think we should add the similar version field to the TPU protocol too.

dimxy avatar Aug 14 '24 15:08 dimxy