forked from jl777/SuperNET
-
Notifications
You must be signed in to change notification settings - Fork 100
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
feat(utxo-swap): add utxo burn output for non-kmd taker fee #2112
Open
dimxy
wants to merge
120
commits into
dev
Choose a base branch
from
burn-dex-fee-for-non-kmd
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 20 commits
Commits
Show all changes
120 commits
Select commit
Hold shift + click to select a range
f3625e5
make mockable optional
dimxy d3f843d
add support for utxo coin burn account to dex fee
dimxy bc5c005
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 7894f7d
remove left over dup dex_fee_rate()
dimxy 2984c67
fix coins crate mocktopus dependency for mm2_main crate tests
dimxy 128c4b4
fix dex fee changed due to new burn output in couple of lp_swap tests
dimxy 23f2a08
fix swap v2 test non-kmd burn output
dimxy 2c810b0
fix fmt
dimxy af45b71
remove comment
dimxy 0e2869e
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 0f34d2e
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy d53dc37
fix zombie chain scan block num
dimxy 81263e0
add burn output for privacy coin,
dimxy 7719b80
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 9331e40
use custom log! instead debug! (as env_logger disabled) in qrc20 dock…
dimxy b343275
fix to make should_burn_dex_fee explicitly in each coin (to fix qrc20…
dimxy 587b1ae
restore original test log message
dimxy 58d84cf
add test for std dex fee for zcoin
dimxy 1243935
remove unused cache dir var in z_coin init block
dimxy bc00ee8
fix clippy
dimxy 4d59545
fix burn addr in ZOutput
dimxy dc9e710
refactor dex fee code;
dimxy 7a2399c
refactor dex fee code to eliminate extra if-else
dimxy 4625554
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy b84b3a9
fix comments (by review notes)
dimxy 35b04a2
refactor var names on review notes
dimxy 8b1033e
refactor fee_amount var names to dex_fee (as those are structs)
dimxy 1a48a04
add test for qtum dex fee with burn output validation
dimxy 06e36b5
remove todo for qrc20 with burn test (as we decided not to add evm bu…
dimxy 1e79216
fix fn should_burn_dex_fee uninmlemented for solana
dimxy 9fd47a4
fix restore cach_db_path (deleted by mistake)
dimxy 217a313
fix review notes: delete unused fn, add cfg for test code
dimxy f35be00
fix review: add test from todo
dimxy 26552c3
add NegotiationDataMsg V4 with version field, add swap features activ…
dimxy c02253a
add non-kmd burn activation with negotiation msg V4
dimxy fd5b4fe
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 22b5c42
added NegotiationDataMsgVersion, removed NegotiationDataV4 (bcz V2 is…
dimxy 9951626
fix activation ver
dimxy d72a313
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy a63c3e0
fix after merge
dimxy 983d8ef
remove relaxed dex fee validation (in favour versioned check)
dimxy 849b190
add mocktopus to mm2_main
dimxy 8e2fdaf
fix version in recreate swap
dimxy b844318
fix node version determination: try versioned then old negotiation me…
dimxy ddb4863
add test feature to emulate old non-versioned negotiation msgs (also …
dimxy 9a34268
fix fmt
dimxy f8b543b
fix swap tx spend size
dimxy 44a2586
move version activation swap_feature mod to coins (to allow using it …
dimxy e6fe67f
add delay between broadcasts of new and old messages
dimxy b20e679
fix old (non-versioned) taker and maker emulation (for tests)
dimxy e930f58
fix swap proto version: use new topic for versioned negotiation (Swap…
dimxy 80d34f6
fix comment
dimxy 5cc210e
add DexFee::NoFee option support for utxo if taker is dex pubkey (dex…
dimxy 69cbfa4
add tests for swap v1 mycoin to mycoin with dex pubkey as Alice (to t…
dimxy 248536a
fix tests to validate fee and recreate swaps
dimxy b0fabcf
fix DexFee::NoFee taker payment spending outputs for swaps V2
dimxy f6cc684
fix tests for no dex fee for swaps v1, add tests for no dex fee for s…
dimxy 7053e2e
fix TODO for delay in broadcast msgs
dimxy 1def60d
fixes for review notes
dimxy 74d8ea3
fixes for review notes
dimxy bc560b8
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy c4b494a
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 347093d
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy f95b91e
add dex fee burning for tendermint coins
dimxy e19b738
fix should_burn_dex_fee to true and burn_pubkey for validate dex fee …
dimxy 446b3db
add mockable support for TendermintCoin (note: some TendermintCoin co…
dimxy 8aa7a06
refactor validate_taker_fee_test with mocked txns
dimxy 53a553f
add test to validate taker fee tx with burn for tendermint
dimxy 2f47bdc
add test to calc DexFee with burn for nucleus ticker
dimxy a61d4a0
Refactor dex fee code:
dimxy 63ecd0b
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 29ad460
fix await in zhtlc tests
dimxy 2ea8b9f
add TODO zhtlc feature
dimxy ab4f2a5
fix fmt
dimxy 8711131
add comments how to run zcoin native tests
dimxy bf65293
add print into tests with hint how to sync wallet db
dimxy 5f0d3d6
add debug logging for updating zcoin wallet db
dimxy b3eccf8
add scan interval in zcoin tests to prevent 'Can't assign requested a…
dimxy d5b9bbd
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy b92d065
fix review notes
dimxy 601519d
fix build err
dimxy 05feb72
add swap feature deactivation by version
dimxy ce1a5e0
improve comments for protocol version
dimxy dcfdbbd
fix burn_pubkey as taker in DexFee::NoFee;
dimxy dfb9673
fix fmt
dimxy 2559e0e
fix tests with burn key as taker
dimxy 70a8bd0
add real burn addr pubkey
shamardy 0a36a09
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 51b58d7
fix: remove compatibility with old swap nodes (no need this anymore)
dimxy 1e24dcf
fix test to reject too old nodes
dimxy ea4d046
temp disable burn dex fee for zcoin
dimxy f321ac0
fix tendermint test burn tx
dimxy afb417e
del temp tx hex logging
dimxy 9a801c2
remove old comment
dimxy ab96ba8
remove unused code to broadcast multiple msgs
dimxy bbe2b5b
fix after merge (remove solana src)
dimxy 897aa4d
refactor (laruh): rename version fields
dimxy e4e8de0
remove negotiation msg version (to be impl in a different way)
dimxy 0b37b10
remove unused features
dimxy de4394f
refactor test mock def, removed unused mockable
dimxy af3762b
cargo fmt
dimxy de079ba
add test helper (mariocynicys)
dimxy a505386
review (borngraced): dex fee errors improved, make value underflow ch…
dimxy 019209f
review (mariocynicys): refactor dex fee dust handling
dimxy d7e18d4
review (borngraced): use as_ref for tendermint address
dimxy 30a13fa
review (mariocynicys): add dex_address, burn_address fn for utxo
dimxy 4b1b867
add flavor multithread to zcoin tests
dimxy 1dcb774
update zcoin txns for real dex and burn address
dimxy 114cfe9
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy 78033be
clear version leftovers in test files
dimxy 89e23ae
remove unused const
dimxy 40ef614
remove unused error
dimxy 2fcda6c
removed unused AddressDerivationPath
dimxy 0cab109
eliminated dex_fee_from_taker_coin
dimxy 2ef2bb6
delete oneinch leftover file
dimxy 07986cb
review (mariocynicys): move TestCoin to for-tests feature, use for-te…
dimxy 56f13b2
review (mariocynicys): add taker_pubkey_bytes fn to CommonSwapOpsV2 f…
dimxy 8513c3f
add doc to DEX_FEE_ADDR_PUBKEY
dimxy 05488f9
add more sepolia nodes for tests
dimxy 703e887
Merge branch 'dev' into burn-dex-fee-for-non-kmd
dimxy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
maybe we could add it as a
for-test
feature.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 the way like it is now is convenient as we may use mocktopus both from tests inside the crate and from other crates, without extra features, and at the same time avoiding building it into releases
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.
from tests inside the crate -> dev-deps section
from tests from outside/integration ->
for-test
flag to activate it, this has the downside though as u said will enable all and every extra test features we have, but it's a for-test build anyway so i wouldn't worry much about that and instead prioritize having less complexcfg
s around the code base.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, in this case mocktopus would be built into release what is not recommended for performance reasons.
The optional flag is a way to avoid this.
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.
im going over my old comments before reviewing so excuse me if things changed and aren't relevant anymore, you can resolve them then.
No it wouldn't make it to
release
. It will only get included whenfor-test
is enabled.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.
As can see, feature 'for-tests' is used to activate certain test code in certain packages.
We may or may not need mocktopus with this test code.
Also, with this approach, if we decide to use mocktopus in other dependency packages we would need to define an extra 'for-tests' feature there, instead simply using features=["mocktopus"]
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.
if we don't need it, then it's a bloat. but who cares, that's a for-test build of mm2 no body will use it.
no, we can simply use
features=["for-tests"]
just like (instead of)feautres=["mocktopus"]
.my problem is that having so many features gets things complicated. and these features aren't even real features that get baked into release, they are only test used features.
why have a feature
x
to bake this functionality into mm2 and another featurey
to bake another functionality and so on. and only activate featuresx
when we are running from this testing crate, and only activatey
when we are doing that.we can instead have a
build-mm2-with-all-test-features
/for-test
feature to enable all the functionalities we need for testing from wherever.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.
To use mocktopus in some subpackage with 'for-tests' you would need to define 'for-tests' in each such subpackage (if it was not defined yet there), although dep:mocktopus is natively defined, w/o extra config and ready-to use.
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.
im lost in the convo, but this is what i mean, might be clearer in code: d5d2bbd
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.
Yes, this is what I meant too:
For e.g. there is a new trading_api package where mocktopus is activated from mm2_main:
komodo-defi-framework/mm2src/mm2_main/Cargo.toml
Line 132 in ff0eefc
Doing it this way it's no need to define an extra [for-tests] feature in trading_api