Skip to content
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

internal: Add basic simnet xmr swap. #2885

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

JoeGruffins
Copy link
Member

closes #2754

@JoeGruffins JoeGruffins force-pushed the xmrscripts branch 5 times, most recently from 815ed6c to 7076e42 Compare August 9, 2024 10:06
@JoeGruffins JoeGruffins force-pushed the xmrscripts branch 7 times, most recently from d60bc68 to bb8eedc Compare August 16, 2024 08:44
@JoeGruffins JoeGruffins marked this pull request as ready for review August 16, 2024 08:54
Copy link
Contributor

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the swap and it works, but the code is a bit hard to understand. I think it would be better if there were two separate clients, initiatorClient and participantClient each with just the functions that they perform. Then, these functions could take as arguments types such as genKeysMsg, genLockTxMsg, etc. which would be equivalent to the communication that would take place when the trade is being done in the dex.

internal/cmd/xmrswap/main.go Show resolved Hide resolved
Comment on lines +87 to +91
kbvf, kbsf, kbvl, kbsl, vkbv *edwards.PrivateKey
pkbsf, pkbs *edwards.PublicKey
kaf, kal *secp256k1.PrivateKey
pkal, pkaf, pkasl, pkbsl *secp256k1.PublicKey
kbsfDleag, kbslDleag [libsecp256k1.ProofLen]byte
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's really hard to follow the code with these variable names. Something like ourSpendKey, counterPartySpendKey, etc. would make it way more understandable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better clean these up on top of #2936? No reason to create conflicts there imo.

@JoeGruffins JoeGruffins force-pushed the xmrscripts branch 2 times, most recently from fc1f63d to d6b0917 Compare August 28, 2024 08:17
JoeGruffins and others added 6 commits September 24, 2024 11:28
Add a c library that has some primitive cryptographic functions needed
for working with adaptor signatures.
	get transaction development details from monerod
	including tx lock time; which is different from
	'locked'/'unlocked' which refer to when a tx
	has 10 confirmations. Tx locktime can be any
	number of blocks in the future.
	- Rename monero_functions.inc -> monero_functions
	- Tidy extra whitespace at eol for harness.sh &
	  monero_functions.
	New monero-wallet-rpc server with no attached wallet. This is
 	for programmatically creating/generating and using a new wallet.

	The wallet will be generated in "own" dir but can be named what-
	ever you need: "alice", "Bob", etc.
"github.com/haven-protocol-org/monero-go-utils/base58"
)

// TODO: Verification at all stages has not been implemented yet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO still valid?

Copy link
Member Author

@JoeGruffins JoeGruffins Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are not inspecting the on-chain txn at all yet. In the end we will get a lot of the info from transactions, not straight from the other party.

Copy link
Contributor

@dev-warrior777 dev-warrior777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dex/networks: Add xmr and dcr adaptor signature primitives.
5 participants