Add federated block-signing server implementation #4
Conversation
The block signer has its own thread to manage active roles in the federation protocol. Most state transitions happen in response to messages received over the network, and are therefore processed in the network thread. However the master node for a round is responsible for kicking off the round with a block proposal, and it is the block-signer thread that does that.
…acks, and signatures.
src/federation.cpp
Outdated
| g_our_block_signing_key = pk; | ||
| } | ||
| #endif // ENABLE_WALLET | ||
| } | ||
|
|
||
| if (!g_our_block_signing_key.IsValid()) { | ||
| LogPrintf("Error: unable to determine block-signing key.\n"); | ||
| return false; | ||
| } | ||
|
|
||
| std::sort(g_block_signing_keys.begin(), | ||
| g_block_signing_keys.end()); | ||
|
|
||
| g_our_block_signing_key_index = 0; |
There was a problem hiding this comment.
It seems like you could move the above wallet-check into the post-sorted loop below, and set both index and key. You could even move that entire thing into an ENABLED_WALLET ifdef, unless the plan is to somehow have a pubkey but not a privkey for our own key (external HWW signer or something).
There was a problem hiding this comment.
That's precisely what I had in mind for production: a pubkey set as a configuration option, with the privkey stored within a secure hardware enclave or attached hardware wallet. I haven't really worked out the low-level details of this though. For example, do we keep the wallet enabled and add the pubkey as a watch-only key? Or do we remove the wallet and replicate the features we need for a smaller exposed attack surface? Honestly I haven't spend more than 5 minutes thinking about that yet. Obviously right now if you try to run this without the wallet enabled it will error out because it can't find any keys it recognizes.
There was a problem hiding this comment.
I moved the wallet-check into the post-sorted loop as suggested because I think that's cleaner. Thanks for the suggestion. I'm not marking this as resolved yet though because of the other unresolved aspects.
src/federation.cpp
Outdated
| if (!GenerateBlockProposal()) { | ||
| LogPrint(BCLog::FEDERATION, "%s: Unable to generate block proposal. Terminating round early.\n", __func__); | ||
| int64_t sec_since_epoch = nextblocktime.time_since_epoch().count(); | ||
| g_roundno = sec_since_epoch / 5; |
There was a problem hiding this comment.
I am assuming several things here, but does this define the current round as the block time in seconds divided by 5 modulo member count? If so, won't this have a pretty poor distribution when there are e.g. 12 peers and 1 minute per block, since the peer will tend to be the same one each time? (12*5 = 60 s = 1 min = block interval).
There was a problem hiding this comment.
This is a mistake. We should be dividing by the length of the period (60 seconds) to get the current round, of course. During development I had this shortened to 5 seconds so I didn't have to sit around waiting for blocks to be generated. Looks like I accidentally committed that constant change at this point.
We should make the round parameters configurable, but that's a larger issue.
There was a problem hiding this comment.
Fixed the constant. Not marking as resolved until we decide how to make this configurable.
There was a problem hiding this comment.
Ahh, that makes sense. Can't you use Params().GetConsensus().nPowTargetSpacing directly?
There was a problem hiding this comment.
Well yes, that would be the obvious thing to do. Duh.
…ons.' nit: netaddrs -> m_netaddrs
…ons.' nit: C++11-isms
…ons.' Move the search for our public key to the second post-sort loop.
…ock-signer.' Switch -blocksignnode to use pubkey@ip:port syntax.
…ons.' Switch -blocksignnode to use pubkey@ip:port syntax.
…ons.' Make code clearer at the small cost of repeated op[] calls.
Remove FIXME note that doesn't need to live in the code.
Fix code style nit
Remove redundant code.
…s, acks, and signatures.' Terminate round early if we're not willing to sign the block.
Reduce redundant code copied from HandshakePeer
…s, acks, and signatures.' Fix incorrect round length
…ons.' int -> size_t to avoid compiler warnings
Pass the same block height to IncrementExtraNonce which was used to construct the block template in the first place.
…egtest network.' Add subsidy coin so that there are coins to spend to create assets.
This pull request adds a federated block-signer built into the Elements daemon. To use, load the wallets of N elements daemons with the signing keys for for a M-of-N proof script, and configure the nodes to talk to each other with
-blocksignnode=. For a detailed example, seecontrib/federation/blocksign.sh.This pull request is a work-in-progress. The federation peer-to-peer code and block-signing capability works, but is currently in the form of a minimal proof-of-concept. There is potentially a lot more work that needs to be done to make it both robust and secure. In addition, to be used in production will probably require some sort of hardware wallet signing support.