-
Notifications
You must be signed in to change notification settings - Fork 101
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
asset: add a backend for Bitcoin #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nits so far.
blockPoll := time.NewTicker(blockPollInterval) | ||
defer blockPoll.Stop() | ||
addBlock := func(block *btcjson.GetBlockVerboseResult) { | ||
_, err := btc.blockCache.add(block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of the polling used to check for new blocks, it's inevitable that blocks will get skipped when blocks are mined faster than the polling interval. Both add
and reorg
should be able to deal with gaps in the chain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gaps are okay, right? No need to pull more than we need. A reorg check is always run when the reported best block doesn't build on our local best block, but the reorg check does not attempt to fill the gap. It starts starts by checking the mainchain status of the local best known block, and goes on checking down from there only if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just making sure that's the intention. Since it's just keeping the block cache correct, not necessarily complete, it's all good as you've described.
|
||
client, err := rpcclient.New(&rpcclient.ConnConfig{ | ||
HTTPPostMode: true, | ||
DisableTLS: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can bitcoind not work with TLS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. They use a "cookie" file, which is just some randomness created at startup and deleted on shutdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's OK I suppose, although in production it will almost certainly be necessary to run the chain daemons on separate machines, which likely means secure network connections will be required... a LAN, VPN, or tunnel can solve that easily enough though.
Adds an asset backend for bitcoin. The implementation is basically a modified version of the DCR backend, with some additional functionality that should allow the backend to be reused for at least some subset of bitcoin forks. The client is btcd/rpcclient, but testing has been against bitcoin core. Live tests are implemented via exported functions that can be used for testing clone compatibility as well. I've added a litecoin backend to demonstrate the clone usage. The litecoin backend, without tests and comments, is < 40 loc.
Adds an asset backend for Bitcoin. The implementation is basically a modified version of the DCR backend, with some additional functionality that should allow the backend to be reused for at least some subset of bitcoin forks. The client is btcd/rpcclient, but testing has been against bitcoin core. Live tests are implemented via exported functions that can be used for testing clone compatibility as well.
I've added a Litecoin backend to demonstrate the clone usage. The Litecoin backend, without tests and comments, is < 40 loc.
Resolving issue #4