Closed pseudozach closed 1 year ago
Thanks for submitting a grant proposal. Our team will review your submission and get back to you.
Hi @pseudozach! We love this idea and would like to better understand the kind of support you’ll need to successfully complete this. Can you please schedule a quick call with Marvin to https://calendly.com/marvin-stacks.
Calendly link shows no available slots, I'll try to reach out to Marvin via Discord and discuss.
Congratulations. Your grant is now approved. Please complete the on-boarding link here: https://stacks-grant.netlify.app/onboard?q=896d058f896dbf92fb5189578a20c150
Hi, stxswap clarity contract is deployed to testnet (v3): https://explorer.stacks.co/txid/0xff7f57b68d1606502c602da608439b6ac1b9d2c6d9480332f23bf4ec888ad0cd?chain=testnet
This concludes M1, I'll continue on M2. Thanks,
@pseudozach nice work! some of the comments on the testnet contract still refer to Eth, I guess that should be STX?
Question.. could you extend this for SIP10 tokens in general? Would be great to have LN invoices paid with Arkadiko's stablecoin USDA
@pseudozach nice work! some of the comments on the testnet contract still refer to Eth, I guess that should be STX?
Question.. could you extend this for SIP10 tokens in general? Would be great to have LN invoices paid with Arkadiko's stablecoin USDA
yes, I've noticed those after I deployed :), v2 will not have those references and I've also found a few potential improvements so I'll deploy v2 and update here (edit: done).
The concept is certainly adaptable to SIP10 token exchange but SIP10 - LN swap contract would need to be developed, and backend/frontend would need work as well. maybe I can add that as M4, @RaffiSapire would that be ok?
If approved, I can add a new milestone as below to support SIP10 swaps:
M4
SoW: Create and deploy a new swap contract that supports SIP10 tokens. Update backend server Implementation to support SIP10 tokens. Update frontend to fetch rates and enable SIP10 tokens.
Deliverables: Contract deployed, Server & Client code pushed to github, deployed to testnet/mainnet.
Approx. Timeline: 1 month/100 hours
Grant Amount: $4000
I'm happy to share a working demo of a successful submarine swap going from STX (mocknet) -> Bitcoin on Lightning Network (regtest): https://youtu.be/2GkRO_KLw4k
Work on M2 and M3 is ongoing in parallel (and pending approval of M4), although lots of things to cover until it's ready for production, feels good to have a working demo of a sample scenario :)
watched the demo, looks great @pseudozach!
!M1_Complete
Thank you for completing M1. The grant committee will review and confirm completion or send feedback within a week
Hi Zach, I'm leaving some comments and questions on behalf of the grant committee:
claimAddress
does not actually seem to be used. In fact, there is a vulnerability in refundStx
that allows anyone to claim the STX tokens for expired swaps. The tx-sender and the original swap submitter are not checked.stx-transfer?
to a contract-call?
to the token contract?) You can also hit two birds with one stone as the SIP009 and SIP010 transfer
functions are compatible.Reference that can help:
Great video by the way! Very exciting stuff.
Thanks for the feedback Marvin. Please find my answers inline.
Why are you submitting all numbers as buffers and then converting them in Clarity? Is there a specific reason why you do not prepare the right CV types in the client? It would lower runtime cost for the contract.
- This is because submarine swaps work by generating a hash of the provided inputs which needs to be unique and calculate to something noone else can guess, in clarity I was able to accomplish this by using keccak256 which accepts inputs of
same type
. I went with buffer and ignored claimAddress/refundAddress which will be removed in a later version of the contract.
claimAddress
does not actually seem to be used. In fact, there is a vulnerability inrefundStx
that allows anyone to claim the STX tokens for expired swaps. The tx-sender and the original swap submitter are not checked.
- I've noticed that while testing and already uploaded v3 in the github repo a week ago that doesnt have this vulnerability but I haven't deployed to testnet. I'll probably have v4 including above optimizations before I complete M2 and deploy to testnet for public use.
We do not think adding SIP010 support would take 1 month/100 hours of work. Could you elaborate on the timeline? (Is it much more than going from
stx-transfer?
to acontract-call?
to the token contract?) You can also hit two birds with one stone as the SIP009 and SIP010transfer
functions are compatible.
- I agree in the scope of clarity contract I do not expect a lot of change there but if you want to peek through the codebase to see the amount of tracking that needs to be done for tokens in nursery, contracthandler and contracteventhandler I think you'll agree it takes a lot of effort to cover all scenarios to support tokens.
- So far I was able move a little faster and finish before 1 month but only because I've been spending so much time (probably more than 100 hours) working on this. Feel free to look into the amount of code changes in the commit history
- I'm perfectly OK to leave the SIP10 token integration to the community, of course anyone will be welcome to prepare support for it and I'll be happy to merge. I can also update M4 scope to 60 hours ($3000) so I can probably complete it a bit sooner now that I'm more familiar with the codebase.
Thanks,
Thanks @pseudozach - we are disbursing for M2.
- This is because submarine swaps work by generating a hash of the provided inputs which needs to be unique and calculate to something noone else can guess, in clarity I was able to accomplish this by using keccak256 which accepts inputs of
same type
. I went with buffer and ignored claimAddress/refundAddress which will be removed in a later version of the contract.
I see. And there is no way around that?
- I've noticed that while testing and already uploaded v3 in the github repo a week ago that doesnt have this vulnerability but I haven't deployed to testnet. I'll probably have v4 including above optimizations before I complete M2 and deploy to testnet for public use.
Looks good. Why do you have (default-to 'ST15RGYVK9ACFQWMFFA2TVASDVZH38B4VAV4WF6BJ ...
in there?
- I agree in the scope of clarity contract I do not expect a lot of change there but if you want to peek through the codebase to see the amount of tracking that needs to be done for tokens in nursery, contracthandler and contracteventhandler I think you'll agree it takes a lot of effort to cover all scenarios to support tokens.
- So far I was able move a little faster and finish before 1 month but only because I've been spending so much time (probably more than 100 hours) working on this. Feel free to look into the amount of code changes in the commit history
- I'm perfectly OK to leave the SIP10 token integration to the community, of course anyone will be welcome to prepare support for it and I'll be happy to merge. I can also update M4 scope to 60 hours ($3000) so I can probably complete it a bit sooner now that I'm more familiar with the codebase.
Interesting! I had no idea that adding additional support had so many implications. I will bring this into the next committee meeting.
- This is because submarine swaps work by generating a hash of the provided inputs which needs to be unique and calculate to something noone else can guess, in clarity I was able to accomplish this by using keccak256 which accepts inputs of
same type
. I went with buffer and ignored claimAddress/refundAddress which will be removed in a later version of the contract.I see. And there is no way around that?
This is the only way I could think of that accomplished submarine swaps, with all the security benefits and using the types in clarity. I was hoping I could convert principals <-> buffer as well but that doesn't seem possible at the moment.
- I've noticed that while testing and already uploaded v3 in the github repo a week ago that doesnt have this vulnerability but I haven't deployed to testnet. I'll probably have v4 including above optimizations before I complete M2 and deploy to testnet for public use.
Looks good. Why do you have
(default-to 'ST15RGYVK9ACFQWMFFA2TVASDVZH38B4VAV4WF6BJ ...
in there?
Just as a fallback account that's for sure not controlled by either party, no special meaning. But now that you mentioned it, I guess it would make sense to have an impossible principal there for added safety. Any suggestions on what I can use?
- I agree in the scope of clarity contract I do not expect a lot of change there but if you want to peek through the codebase to see the amount of tracking that needs to be done for tokens in nursery, contracthandler and contracteventhandler I think you'll agree it takes a lot of effort to cover all scenarios to support tokens.
- So far I was able move a little faster and finish before 1 month but only because I've been spending so much time (probably more than 100 hours) working on this. Feel free to look into the amount of code changes in the commit history
- I'm perfectly OK to leave the SIP10 token integration to the community, of course anyone will be welcome to prepare support for it and I'll be happy to merge. I can also update M4 scope to 60 hours ($3000) so I can probably complete it a bit sooner now that I'm more familiar with the codebase.
Interesting! I had no idea that adding additional support had so many implications. I will bring this into the next committee meeting.
Thanks, let me know the feedback from the committee. This was already quite an undertaking but so far good progress and I personally find it cool to work on.
Just as a fallback account that's for sure not controlled by either party,
Something like (as-contract tx-sender )
?
I think that'd have the same problem that refund could be collected by the lnstxbridge operator, right? Is there an account with an impossible private key? or maybe there's a better way to handle this in clarity I'm not thinking of.
Hey @pseudozach, how is progress on M2 and M3 going? Do you have an approximate timeline for when you plan to finish both milestones and is there anything we can do to help you push them over the finish line?
I see that you've proposed adding a 4th milestone above. Would you mind briefly summarizing your idea for M4 in a new comment, just to make it easier for the community to see what your proposal is?
Hi Jenny, app backend even frontend is mostly finished, for the past 2 weeks I've been struggling to bring up a testnet stacks-node and api because I need to subscribe to contract events for the app to work. Unfortunately my testnet instance keeps getting stuck and not reaching the chain tip.
root@localhost:~# curl -sL localhost:3999/v2/info | jq . { "peer_version": 4207599105, "pox_consensus": "25453ed5b77b48328a6a44e1e97a18e0b43d8e72", "burn_block_height": 2034651, "stable_pox_consensus": "0bf090ac81c99ed39adbbd13a341ac2ad5f979a2", "stable_burn_block_height": 2034644, "server_version": "stacks-node 2.0.11.2.0 (master:63d24b1+, release build, linux [x86_64])", "network_id": 2147483648, "parent_network_id": 118034699, "stacks_tip_height": 0, "stacks_tip": "0000000000000000000000000000000000000000000000000000000000000000", "stacks_tip_consensus_hash": "0000000000000000000000000000000000000000", "genesis_chainstate_hash": "74237aa39aa50a83de11a4f53e9d3bb7d43461d1de9873f402e5453ae60bc59b", "unanchored_tip": "0000000000000000000000000000000000000000000000000000000000000000", "unanchored_seq": 0, "exit_at_block_height": null }
I've gotten some support through discord but still stuck on this. As soon as I can sync a testnet node, I can share public URL for the app so everyone can access the app, which should close M2.
After a week or so testing/feedback/improvements I can push to mainnet and close M3 as well.
Thanks for offering support, I keep working on it and I'll ask my questions on discord testnet channel. My understanding is that I just need to keep trying, because I use all the latest config and sw versions.
Here's my original proposal for M4 to be added to this grant if approved:
Thanks to jw from hiro team on discord I was able to sync the testnet node.
App is now available for public testing at https://ss.pseudozach.com swaps are available between Stacks Testnet <-> Lightning on Bitcoin Testnet
Both backend and frontend code is pushed to github with instructions https://github.com/pseudozach/lnstxbridge https://github.com/pseudozach/lnstxbridge-frontend
!M2_Complete
Thank you for completing M2. The grant committee will review and confirm completion or send feedback within a week
Thanks to jw from hiro team on discord I was able to sync the testnet node.
App is now available for public testing at https://ss.pseudozach.com swaps are available between Stacks Testnet <-> Lightning on Bitcoin Testnet
Both backend and frontend code is pushed to github with instructions https://github.com/pseudozach/lnstxbridge https://github.com/pseudozach/lnstxbridge-frontend
!M2_Complete
Hi @pseudozach thanks for the update and for completing M3 early! The app looks great and everything seems be on track so far. Before we fund M3, there are a couple of things we would like for you to work on:
Decision on M4: Instead of adding M4 to the scope of work for this grant, we recommend submitting M4 as a separate grant after you've incorporated the improvements suggested above.
Hi @jennymith thanks for the feedback again,
I've shared the app on discord and the forum so far feedback is very positive. I suppose there will be more users and more high quality feedback once the app is on mainnet.
I'll look forward to hear back on 2nd point from @MarvinJanssen because I feel like the last version of contract is production ready in my view.
here is some feedback for reference:
https://twitter.com/cryptoserrano/status/1445424362534969351?s=20
Podcast interview on LNSwap: https://twitter.com/JakeBlockchain/status/1448395916054986755
https://www.builtonbitcoin.xyz/ep10-connecting-lightning-bitcoin-network-stacks-blockchain-pseudozach-interview/
Hi @pseudozach, definitely ready for use. We simply love this project so maybe that is why we are being a bit extra picky... But to get to the point, we had another internal discussion about the technical implementation. Given that LN is meant to bring instant payments and settlements we feel the contract logic should be as optimal is possible.
Here are some of the findings, would love to hear your thoughts:
Buffers vs numbers
We kept coming back to the fact that the numbers are still passed in as buffers and then converted to native uints. Doing the conversion on-chain instead of off-chain adds a serious runtime cost. I see that those number buffers are used in the hash being stored, but is it a requirement to store the hash that way? If the concern is verifiability, the application can still see the correct numbers were posted to the contract, right? If this is somehow an absolute requirement of LN, can you link me to a resource? If it is not, now that you have a beautiful frontend you could do that conversion there; namely, around here: https://github.com/pseudozach/lnstxbridge-frontend/blob/f02774a69a3bcb8bcabbf15b8cf8126ba2ad13cb/src/views/swap/steps/sendTransaction.js#L214.
Code hygiene and recalculating the same hash multiple times
You call hashValuesbuf
multiple times in the same function, wasting runtime. It is better to set the result as local variable using let
and then reuse it.
There is one structure that I think is worth refactoring. I see what you are doing but embedding a default "random" address might tick off alarm bells for people. This one here:
(asserts! (is-eq claimer (default-to 'ST15RGYVK9ACFQWMFFA2TVASDVZH38B4VAV4WF6BJ (get initiator (map-get? swaps {hash: (hashValuesbuf preimageHash amount timelock)})))) (err u1002))
If you retrieve the swap first and set it as a local variable, you can already cover the situation of a swap not existing.
Then there is also an (is-eq)
with a bool true
.
Here is an example of how refundStx
could be refactored.
(define-public (refundStx (preimageHash (buff 32)) (amount (buff 16)) (timelock (buff 16)))
(let (
(claimer tx-sender)
(calculatedHash (hashValuesbuf preimageHash amount timelock))
(swap (unwrap! (map-get? swaps {hash: calculatedHash}) (err u9999)) ;; some error code if the swap does not exist
)
(begin
(asserts! (> block-height (buff-to-uint-le timelock)) (err u1001))
(asserts! (checkSwapIsLocked calculatedHash) (err u1000))
(asserts! (is-eq claimer (get initiator swap))) (err u1002))
(map-delete swaps {hash: calculatedHash})
(try! (as-contract (stx-transfer? (buff-to-uint-le amount) tx-sender claimer)))
(print "refund")
(print calculatedHash)
(ok u1008)
))
)
Minor key optimisation
The map tuple key only contains one field, it can therefore be simplified to just that field.
;; from:
(define-map swaps {hash: (buff 32)} {locked: bool, initiator: principal})
;; to:
(define-map swaps (buff 32) {locked: bool, initiator: principal})
Error constants
You could consider defining error constants for readability.
(define-constant err-swap-not-found (err u9999))
;; somewhere in the code:
(unwrap! (map-get? swaps {hash: calculatedHash}) (err u9999))
Hi @pseudozach, definitely ready for use. We simply love this project so maybe that is why we are being a bit extra picky... But to get to the point, we had another internal discussion about the technical implementation. Given that LN is meant to bring instant payments and settlements we feel the contract logic should be as optimal is possible.
Here are some of the findings, would love to hear your thoughts:
Buffers vs numbers
We kept coming back to the fact that the numbers are still passed in as buffers and then converted to native uints. Doing the conversion on-chain instead of off-chain adds a serious runtime cost. I see that those number buffers are used in the hash being stored, but is it a requirement to store the hash that way? If the concern is verifiability, the application can still see the correct numbers were posted to the contract, right? If this is somehow an absolute requirement of LN, can you link me to a resource? If it is not, now that you have a beautiful frontend you could do that conversion there; namely, around here: https://github.com/pseudozach/lnstxbridge-frontend/blob/f02774a69a3bcb8bcabbf15b8cf8126ba2ad13cb/src/views/swap/steps/sendTransaction.js#L214.
Code hygiene and recalculating the same hash multiple times
You call
hashValuesbuf
multiple times in the same function, wasting runtime. It is better to set the result as local variable usinglet
and then reuse it.There is one structure that I think is worth refactoring. I see what you are doing but embedding a default "random" address might tick off alarm bells for people. This one here:
(asserts! (is-eq claimer (default-to 'ST15RGYVK9ACFQWMFFA2TVASDVZH38B4VAV4WF6BJ (get initiator (map-get? swaps {hash: (hashValuesbuf preimageHash amount timelock)})))) (err u1002))
If you retrieve the swap first and set it as a local variable, you can already cover the situation of a swap not existing.
Then there is also an
(is-eq)
with a booltrue
.Here is an example of how
refundStx
could be refactored.(define-public (refundStx (preimageHash (buff 32)) (amount (buff 16)) (timelock (buff 16))) (let ( (claimer tx-sender) (calculatedHash (hashValuesbuf preimageHash amount timelock)) (swap (unwrap! (map-get? swaps {hash: calculatedHash}) (err u9999)) ;; some error code if the swap does not exist ) (begin (asserts! (> block-height (buff-to-uint-le timelock)) (err u1001)) (asserts! (checkSwapIsLocked calculatedHash) (err u1000)) (asserts! (is-eq claimer (get initiator swap))) (err u1002)) (map-delete swaps {hash: calculatedHash}) (try! (as-contract (stx-transfer? (buff-to-uint-le amount) tx-sender claimer))) (print "refund") (print calculatedHash) (ok u1008) )) )
- These are super helpful, I've integrated into stxswap_v6, pushed to github, which I'll deploy to mainnet in a few days once mainnet node is ready.
Minor key optimisation
The map tuple key only contains one field, it can therefore be simplified to just that field.
;; from: (define-map swaps {hash: (buff 32)} {locked: bool, initiator: principal}) ;; to: (define-map swaps (buff 32) {locked: bool, initiator: principal})
Error constants
You could consider defining error constants for readability.
(define-constant err-swap-not-found (err u9999)) ;; somewhere in the code: (unwrap! (map-get? swaps {hash: calculatedHash}) (err u9999))
Thanks for the detailed feedback and guidance as always!
How is claim-stx
secured so that the locked stxs can't be claimed by any user? I had a look at v6 on main branch and didn't see anything.
Could you add some Clarinet tests?
How is
claim-stx
secured so that the locked stxs can't be claimed by any user? I had a look at v6 on main branch and didn't see anything.
claimstx works such that claimer needs to send the preimage for the LN invoice as an input to the function. Function then uses preimagehash (sha256 preimage) to calculate the swap of the hash. preimage is, by definition, a secret that is revealed to the payer of an LN invoice upon payment and can not be known by anyone beforehand.
The point is that the preimage is revealed via a transaction. An attacker can watch the mempool for claimStx
calls and try to front-run the user that submitted it. Additionally, malicious miners can purposely ignore claimStx
calls and instead claim those STX themselves. The STX address of the claimant should be set beforehand like in the atomic swap PoC. There might be other attack vectors if the preimage is revealed off-chain to settle the invoice and this is picked up by someone.
That's a valid concern. I suggest the following update:
I'll update the contract, run my regular tests and deploy to testnet.
Update: stxswap_v7 is deployed on testnet and https://ss.pseudozach.com is updated with this version.
Note that I've incorporated a lot of feedback/testing result to the app/contract and frontend is ready as well. As soon as M2 review is completed I can move to mainnet and complete M3.
Hi, I'm happy to announce the first mainnet submarine swap between STX on Stacks and Bitcoin on Lightning Network!
Mainnet contract deployed: https://explorer.stacks.co/txid/SP2507VNQZC9VBXM7X7KB4SF4QJDJRSWHG4V39WPY.stxswap_v7?chain=mainnet Swap Frontend: LNSwap.org
User locked ~2 STX in return for ~5500 satoshis. Swap was successfully completed, user received the LN payment revealing the preimage and Swap operator (lnswap.org) claimed the STX from the contract to finalize the swap.
Here's the mainnet demo video: https://www.youtube.com/watch?v=PdEwUFqOGj8
The page does not seem to be accessible, pinging the ip and lnswap.org is returning succes however.
Are the transactions send with post-conditions in "allow" mode intentionally?
Users would get a warning, "~unsafe transaction" before the transaction is send, deny mode is preferred on mainnet.
Yes @314159265359879 sorry, I should've made sure server&nodes are stable before the announcement but I got over excited :) I'll make another announcement when everything is stable and everything is accessible again.
I've noticed unsafe tx warnings on Hiro wallet as well but to be honest I had difficulty running these with deny mode. I'll include this as an improvement for a follow up grant/v2 of the app. That list is only growing.
For the frontend, I see that you don't use connect, nor stacks/transactions. If the libraries do not help, we need to improve the libraries :-)
All nodes/backend/frontend is now up and reachable at https://lnswap.org. I've also completed a reverse-submarine swap to ensure everything is OK. LN node has low liquidity so please test with small amounts. Known issues:
ConflictingNonceInMempool
cc @314159265359879 For the frontend, I see that you don't use connect, nor stacks/transactions. If the libraries do not help, we need to improve the libraries :-)
I do use connect and @stacks/transactions in both views that user clicks to start a contract call here: https://github.com/pseudozach/lnstxbridge-frontend/blob/main/src/views/swap/steps/sendTransaction.js#L15
For other details (postconditions etc) I've commented on the repo issue with more details: https://github.com/pseudozach/lnstxbridge-frontend/issues/1
!M3_Complete
Thank you for completing M3. The grant committee will review and confirm completion or send feedback within a week
The page does not seem to be accessible, pinging the ip and lnswap.org is returning succes however.
Are the transactions send with post-conditions in "allow" mode intentionally?
Users would get a warning, "~unsafe transaction" before the transaction is send, deny mode is preferred on mainnet.
This is fixed, contract calls use deny mode now: https://github.com/pseudozach/lnstxbridge-frontend/issues/1#issuecomment-933939577
Copied from telegram, I thought you'd like to know. I will ask him to post here when it is resolved.
This user still has an issue on the second attempt the claim button is unresponsive. He is on Windows 10 and the latest Chromium Edge browser.
He also says thanks pseudozach for making it possible for him to get his first STX.
![]()
![]()
This user still has an issue on the second attempt the claim button is unresponsive. He is on Windows 10 and the latest Chromium Edge browser.
He also says thanks pseudozach for making it possible for him to get his first STX.
thanks to yours and another user report on discord, I've identified and resolved this issue. This is the relevant commit: https://github.com/pseudozach/lnstxbridge-frontend/commit/9ae50b2a1bbfcefb96f833f4c96323deac7a8da1
we've fixed a number of issues and pushed out a new contract version thanks to many users' reports: https://github.com/pseudozach/lnstxbridge/issues/10 I will also improve the frontend with more transparency on the number of swaps & volume info. Open to any other suggestions/feedback on the relevant github repos.
Background What problems do you aim to solve? How does it serve the mission of a user owned internet?
Project Overview What solution are you providing? Who will it serve?
As part of this grant a new submarine swap implementation will be created and open sourced. This service will run an API that's reachable publicly and a front-end will serve non-custodial swaps between STX and bitcoin on Lightning Network.![autodraw 7_19_2021](https://user-images.githubusercontent.com/3731591/126237553-e7ef543a-5051-42b0-a712-a765f2d5538c.png)
This service will serve users that want a quick & non-custodial way to swap and services that want to accept other coins but do not want to deal with custody.
For instance this tweet, on integrating LN payments for Stacks apps, will be easily answered by integrating with this API: https://twitter.com/muneeb/status/1400134618230185985?s=19
Scope What are the components or technical specs of the project? What will the final deliverable look like? How will you measure success?
Clarity contract that implements all required methods for submarine/reverse-submarine swaps
Backend server/API that manages an LN node, tracks payments to the clarity contract and executes swaps/refunds in case of failures in a non-custodial manner.
Frontend that makes it easy to complete swaps by preparing contract calls and guiding regular users through swap steps.
Once all the components are developed and deployed, we'll track usage if there's more demand for regular users to do non-custodial swaps or check if there's demand from both STX and LN app developers if an API is more useful, and focus accordingly.
Budget and Milestones What grant amount are you seeking? How long will the project take in hours? If more than 20, please break down the project into milestones, with a clear output (e.g., low-fi mockup, MVP with two features) and include the estimated work hours for each milestone.
Total Grant Request: $10000
Team Who is building this? What relevant experience do you bring to this project? Are there skills sets you are missing that you are seeking from the community? Please share links to previous work.
Risks What dependencies or obstacles do you anticipate? What contingency plans do you have in place?
Community and Supporting Materials Do you have previous projects, code commits, or experiences that are relevant to this application? What community feedback or input have you received? How do you plan to share your plan to the community over time and as the final deliverable?