search

AustEcon / bitsv

BitSV: Bitcoin made easy. Documentation:
https://AustEcon.github.io/bitsv
MIT License
96 stars 28 forks source link

simple network service for local full-node #35

Closed xloem closed 5 years ago

xloem commented 5 years ago

Hi,

I made a basic backend for connecting the API to a locally running node via jsonrpc, rather than a web service.

I've used this locally to successfully upload a file with polyglot, but I haven't added it to NetworkAPI here as I wasn't sure of the interface for this. It passes the testsuite (if 3 nodes are all running locally for the 3 different network types), but I haven't manually tested anything or inspected whether the testsuite covers everything I wrote.

AustEcon commented 5 years ago

Awesome! Thanks so much. I will need a bit of time to take a closer look. And probably Teran too. But my first impression (from my phone) is that I like it. There have been requests for this kind of thing + Reg test.

Thanks!

ghost commented 5 years ago

Hey! This is very cool but I feel bad that you reinvented the wheel over this. Have you seen the pull request in bit where this is implemented? I imagine it would copy over almost identically.

https://github.com/ofek/bit/pull/76

ghost commented 5 years ago

By the way, we're generally dropping cashaddr, I need to pull more of those bits out. No need to support it in something new.

xloem commented 5 years ago

Thanks, I didn't really know that bitsv was forked from bit. It looks like the bit PR is more polished. My code handles some transaction cases it doesn't, but lacks the docs etc.

Regarding cashaddr, the bitcoin sv node presents addresses in cashaddr format only. So it's needed for compatibility with the service for now.

AustEcon commented 5 years ago

To be honest. I like this implementation better than the bit one. I don't have strong feelings either way on it though. I find this more readable and minimalist. (Which I like).

I think the "connect_to_node" classmethod idea in NetworkAPI section is a good way to override the default and switch over to using the bitcoin node... But with the way we have set it up as instantiating an object... I think we would do this a bit differently too.

I'm not sure how is best yet though... maybe a parameter in PrivateKey constructor of use_local_node=True and full_node=FullNode_instance? and if the full_node parameter is left blank (None)... should try to make it work by default

ghost commented 5 years ago

Bitcoin SV node returns cashaddr? That's terrible, needs to get fixed. Good to know.

I was just curious for your thoughts on that implementation and to see if it it solved any issues for you. I can review both and give you feedback, just wanted you to see it first.

AustEcon commented 5 years ago

Cashaddr is going to be removed soon. https://twitter.com/BSVGlobal/status/1149737932149989378?s=20

xloem commented 5 years ago

That’s great they resolved the cashaddr thing. I’m away from the Internet for the weekend and can’t work on this more myself until some future day.

AustEcon commented 5 years ago

I've really been wanting to cut a new release (ideally tomorrow). So what I might do, is leave this fullnode implementation out of this next release but I can follow it up in quick succession with a new minor version update.

ghost commented 5 years ago

Yeah, I would love to have full node support but I think a minor release before that would be good.

ghost commented 5 years ago

Hey! We've cut our releases. I'd love to see local node support added to bitsv.

Also, node support was merged into bit. Might be worth taking one more look at even just for ideas.

https://github.com/ofek/bit/pull/76

AustEcon commented 5 years ago

Sorry I took so long with this... I made some modifications because of all of the recent changes that have broken this PR.. and then just decided to merge. I will close this now.

Thanks so much for this. Great addition :smiley: