rust-bitcoin / rust-bitcoind-json-rpc

Support for Bitcoin Core's JSON-RPC API
Creative Commons Zero v1.0 Universal
7 stars 4 forks source link

Import `jsonrpc` crate #19

Closed tcharding closed 2 weeks ago

tcharding commented 3 weeks ago

Import the rust-jsonrpc crate from https://github.com/apoelstra/rust-jsonrpc using current tip of master

59646e6 Merge apoelstra/rust-jsonrpc#119: Use rust-bitcoin-maintainer-tools and re-write CI

Full commit hash: 59646e6e6ac95f07998133b1709e4a1fa2dbc7bd

Do so using the following commands:

mkdir jsonrpc mkdir jsonrpc/contrib rsync -avz ../../rust-jsonrpc/master/README.md jsonrpc rsync -avz ../../rust-jsonrpc/master/src jsonrpc rsync -avz ../../rust-jsonrpc/master/contrib/test_vars.sh jsonrpc/contrib

Then:

Finally import fuzzing, and fix up to mimic current rust-bitcoin setup.

tcharding commented 3 weeks ago

Is this an acceptable way to do this? Still todo

tcharding commented 3 weeks ago

cc @apoelstra

apoelstra commented 3 weeks ago

As a strategy for moving jsonrpc that works for me.

Did we agree though to move jsonrpc? I'd have to think a bit about whether that's what we want to do. In theory it's a general-purpose jsonrpc crate, though in practice we've pretty-much ignored bug reports that come from non-bitcoind users. (Although those reports are just "this doesn't support SSL" 95% of the time, and we did fix that.)

tcharding commented 3 weeks ago

Did we agree though to move jsonrpc?

Yeah I believe we did, remember it was the idea put together all the stuff that we feel "obligated by honour" to provide and maintain.

apoelstra commented 3 weeks ago

Ah, yeah, that makes sense. I remember that conversation but I guess it didn't occur to me that it'd mean moving jsonrpc into this repo.

Let's do it!

tcharding commented 3 weeks ago

We can move this repo to rust-bitcoin org first if you'd prefer it to not transition through my user account. That might look better, really.

apoelstra commented 3 weeks ago

Sure -- ACked in the discussion you opened about it. But you need to do the transfer since it's coming from your org.

tcharding commented 3 weeks ago

Hey @apoelstra I'm not sure what to do about fuzzing jsonrpc, the fuzz tests test that rubbish read back from a socket doesn't cause us to crash, which seems like a good test but I don't think we need to run it or an hour like we do in the rust-bitcoin cron job. Do you have a suggestion for how we should go about running it here?

apoelstra commented 3 weeks ago

Is there any harm in running it for an hour? If we're running it on a GA runner that's probably more like 5-10 minutes on a normal computer.

But if you want to change it we could add a variable to our fuzz runner to make this configurable.

tcharding commented 3 weeks ago

Lets run it for an hour then.