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

No std #74

Closed
wants to merge 4 commits into from
Closed

No std #74

wants to merge 4 commits into from

Conversation

DanGould
Copy link
Contributor

@DanGould DanGould commented May 23, 2023

Conform to wasm restrictions for MutinyWallet/mutiny-node#194

bitcoin/rand is a reexport of secp256k1/rand which is no-std

@TonyGiorgio I think these are the changes you're looking for

This is done so that we can support wasm environments
@DanGould DanGould marked this pull request as ready for review May 23, 2023 02:47
@TonyGiorgio
Copy link

I tested this branch out but it looks like that version of bip21 is pulling in std for bitcoin:

├── payjoin feature "send"
│   └── payjoin v0.8.0 (https://github.com/DanGould/rust-payjoin.git?branch=no-std#d79c5493)
│       ├── bitcoin v0.29.2 (*)
│       ├── base64 feature "default" (*)
│       ├── log feature "default" (*)
│       ├── url feature "default" (*)
│       └── bip21 feature "default"
│           └── bip21 v0.2.0
│               ├── bitcoin feature "default"
│               │   ├── bitcoin v0.29.2 (*)
│               │   ├── bitcoin feature "secp-recovery" (*)
│               │   └── bitcoin feature "std"

Maybe try bip21 v0.3.0 ? @benthecarman added no-std support recently: Kixunil/bip21@8113919

@DanGould
Copy link
Contributor Author

Rats, we're waiting on @Kixunil to release a new version of bip21 removing bitcoin default-features

image

@TonyGiorgio
Copy link

Ah, for some reason I thought we were already using that library.

@Kixunil
Copy link
Contributor

Kixunil commented May 24, 2023

This doesn't work since you don't have #[no_std] in lib.rs, after which I expect more things to break, you'll see.

I originally didn't bother with no_std because I expected that payjoin is effectively impossible/impractical without OS but WASM is a good counterargument, so concept ACK.

I will look into releasing BIP21 soon.

@Kixunil
Copy link
Contributor

Kixunil commented May 24, 2023

FYI I came across this: https://users.rust-lang.org/t/is-disabling-features-of-a-dependency-considered-a-breaking-change/94302

Will give it some time for people to reply and decide afterwards.

@DanGould
Copy link
Contributor Author

DanGould commented May 24, 2023

@cryptoquick, How come bitmask-core has no problem with std payjoin and bitcoin crates? Don't you build it with wasm_pack?

@cryptoquick
Copy link
Contributor

Sure, we use std with WASM, works great!

For those curious:
https://github.com/diba-io/bitmask-core

@benthecarman
Copy link

Yeah we have to use no-std for mutiny because ldk expects you to have system time unless you are no std

Close payjoin#70. Some people might want to use a database or a different data structure.
HashMap isn't supported in no-std.
@Kixunil
Copy link
Contributor

Kixunil commented May 26, 2023

@benthecarman IIUC you could just deactivate std in LDK and keep it in payjoin. It should work despite looking weird. And for not looking weird part, based on the discussion I will release a non-breaking release.

@Kixunil
Copy link
Contributor

Kixunil commented May 26, 2023

I've released bip21 0.3.1.

@DanGould DanGould removed the blocked label May 31, 2023
@DanGould
Copy link
Contributor Author

I just published payjoin-0.8.1 with bitcoin-0.30.0 and bip21 0.3.1.

IIUC you could just deactivate std in LDK and keep it in payjoin. It should work despite looking weird. And for not looking weird part, based on the discussion I will release a non-breaking release.

Does this work in your wasm setting @TonyGiorgio? You can see from the updated draft, as Kixunil said, no-std here is more involved than I thought. I'd definitely run a mutinynet payjoin receiver at payjoin.org and a new version of relayed payjoin is getting ready to drop, too.

@TonyGiorgio
Copy link

Cool thanks! It looks like 0.8.1 works when I include it in my cargo and run a build.

TBH some of the rust dependency hell confuses me sometimes. Can't tell if its sometimes just problems with wasm-pack, LDK, or general rust.

mutinynet payjoin receiver at payjoin.org

Would love that! Also been meaning to ask, we've worked out a way to do p2p connections from mobile going through a websocket proxy. If we had two peers talking to each other, would it be possible with this library to get them to do a payjoin when one is paying the other? I haven't looked too deeply at the API's yet but at least for paying a server it should be no problem.

@DanGould
Copy link
Contributor Author

DanGould commented May 31, 2023

If we had two peers talking to each other, would it be possible with this library to get them to do a payjoin when one is paying the other? I haven't looked too deeply at the API's yet but at least for paying a server it should be no problem.

Yes. There are 2 caveats to look out for

  1. the body is unencrypted/unauthenticated, so without something like "serverless payjoin" upgrades you're placing a lot of trust in the coordinator (not to snoop or MITM attack by changing in-flight psbt) and
  2. I'd look out for bip21 compatibility. Ideally whatever you proxy doesn't break that. what I have in mind is the receiver allocating space on a proxy with websockets on a server that listens to http concurrently. That way old bip78 senders can still send to the receivers listening to the relay.

Because of caveat 1, it might be best to release this to production until the encryption & authentication is dealt with, but I'd love to see a mutinynet rollout of a relay like you describe to prove the concept and later have as a fallback

p.s. a final consideration I forgot to mention for a new protocol is asynchrony. It solves so many ux problems.

@Kixunil
Copy link
Contributor

Kixunil commented Jun 1, 2023

Hmm, for a while I was thinking about PayJoin v 2.0 and I'm now convinced we should do it. I will discuss with folks at dev/hack/day. @DanGould we could do some sort of video call I guess.

@DanGould
Copy link
Contributor Author

DanGould commented Jun 1, 2023

Hell yes. I'd like for us to have a v2 document drafted to talk about. I made an unencrypted public payjoin matrix room to organize https://matrix.to/#/%23payjoin%3Amatrix.org

@DanGould
Copy link
Contributor Author

Looks like Mutiny and other wasm projects can use this library without no-std, so I'm closing this to avoid maintaining complexity until it becomes necessary.

@DanGould DanGould closed this Jul 14, 2023
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.

None yet

5 participants