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

Add rust-bitcoin utility functions #83

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

cygnet3
Copy link
Owner

@cygnet3 cygnet3 commented Mar 27, 2024

No description provided.

@cygnet3 cygnet3 force-pushed the rust-bitcoin-dependency branch 2 times, most recently from 3afaeba to 34adb75 Compare March 27, 2024 13:49
@cygnet3 cygnet3 changed the title WIP Add rust-bitcoin utility functions WIP: Add rust-bitcoin utility functions Mar 27, 2024
@cygnet3 cygnet3 marked this pull request as draft March 27, 2024 19:48
@cygnet3 cygnet3 marked this pull request as ready for review April 10, 2024 11:11
@cygnet3 cygnet3 changed the title WIP: Add rust-bitcoin utility functions Add rust-bitcoin utility functions Apr 10, 2024
@Sosthene00
Copy link
Contributor

The commits itself looks fine, but my concern is more about the trade-off of adding a (big) dependency that breaks all the time for the added convenience, compared to the original approach of keeping it out of rust-silentpayment and let downstream consumers (like sp-backend) deal with it. I think it's a bit premature now, and maybe we can talk about it again when sp-backend and donation wallet stabilize.

@cygnet3 cygnet3 marked this pull request as draft May 8, 2024 12:05
@pythcoiner
Copy link

but my concern is more about the trade-off of adding a (big) dependency that breaks all the time for the added convenience,

i'm wondering if it is a way to supply util as macros instead functions, where the consumer can supply it's own bitcoin dependency?

@cygnet3
Copy link
Owner Author

cygnet3 commented Aug 20, 2024

That sounds like it could work indeed, I'll check it out when I have the time

@Sosthene00
Copy link
Contributor

That sounds great, but needs more investigation, I'm not super confortable with writing macros atm

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.

4 participants