-
Notifications
You must be signed in to change notification settings - Fork 94
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
[r2r] cosmos swap p.o.c #1468
[r2r] cosmos swap p.o.c #1468
Conversation
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
@ozkanonur I converted this to draft to prevent additional CI checks. |
Signed-off-by: Onur Özkan <[email protected]>
Pipelines are still getting triggered. Seems like CI definitions needs to be updated for that. I will close the PR for now and re-open once it's ready. An addition, we can also read the PR title and ignore running CI if title starts with |
# Conflicts: # mm2src/coins/lp_coins.rs # mm2src/mm2_main/src/mm2_tests.rs # mm2src/mm2_test_helpers/src/for_tests.rs
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Pass secret hash directly.
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
…PI into iris-swap-poc
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
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.
Few notes from my side.
mm2src/coins_activation/src/tendermint_with_assets_activation.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
# Conflicts: # mm2src/coins/tendermint/tendermint_coin.rs # mm2src/coins_activation/src/tendermint_with_assets_activation.rs # mm2src/mm2_main/src/mm2_tests.rs
Signed-off-by: ozkanonur <[email protected]>
Signed-off-by: ozkanonur <[email protected]>
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.
LGTM.
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.
LGTM, thanks!
384904f
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.
Huge Work 🚀 ! Only 3 comments :)
fee, | ||
} | ||
} | ||
|
||
pub fn new_max(coin: String, to: String) -> WithdrawRequest { |
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 don't think we need this new_max
function also, it's used only once in testing.
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 believe there are some other functions like this as well. Not used or used only once. We can exclude them from source code as another task in another PR.
EthCoinType::Erc20 { .. } => "erc20Payment", | ||
EthCoinType::Eth => { | ||
if secret_hash.len() == 32 { | ||
"ethPaymentSha256" |
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 don't think we really need these new functions ethPaymentSha256
and erc20PaymentSha256
since the H160
secret hash is equal to ripemd160(sha256(secret))
and sha256(secret)
is shared in the NegotiationDataMsg
if either coins are Tendermint
. The H160
secret hash can always be used by the other side of the swap that's not a TendermintCoin
and it can be derived at the TendermintCoin
side.
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.
It's a very good note 🙂 With this, we might avoid updating swap smart contracts at all, and make API code simpler. Added to checklist.
.push_opcode(Opcode::OP_EQUALVERIFY); | ||
|
||
if secret_hash.len() == 32 { | ||
builder = builder.push_opcode(Opcode::OP_SHA256); |
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 comment as above, no need for SHA256
secret hash on utxo side.
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.
Added to checklist, thanks!
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.
Re-approving.
This PR includes implementations of Cosmos based coin/token activations and swap implementations using Iris Network's HTLCs.
p.s: Rest of the todos, and P.o.C work will be refactored/implemented in the next PR. Also, PR requires lots of optimizations(had to rush on this implementation because of cosmosverse event) which I am aware of, in my opinion optimizations should also be part of the next PR.