Nakamoto-Neon: Add nakamoto_node #4133
Conversation
kantai
changed the title
| /// Communication link to the coordinator thread | ||
| pub(crate) coord_comms: CoordinatorChannels, | ||
| /// Unconfirmed transactions (shared between the relayer and p2p threads) | ||
| unconfirmed_txs: Arc<Mutex<UnconfirmedTxMap>>, |
There was a problem hiding this comment.
This struct is shared with the neon_node. I could make this field generic as well (and use a void type for it in nakamoto), but that didn't seem like a clear improvement to me.
| /// Cointer state in the main thread | ||
| pub counters: Counters, | ||
| /// Connection to the PoX sync watchdog | ||
| pub sync_comms: PoxSyncWatchdogComms, |
There was a problem hiding this comment.
This can be dropped for Nakamoto as well -- the system must wait for the missing PoX anchor block in all cases.
There was a problem hiding this comment.
Same as above-- this struct is shared with the neon_node. This field eventually will need to be generic. I think if we don't need the pox_watchdog, this would just be a struct monitoring the mine_after setting (where miners can set when they want to begin issuing block commits).
| &chain_state, | ||
| &burn_db.index_conn(), | ||
| &mut mem_pool, | ||
| // TODO (refactor): the nakamoto block builder doesn't use the parent tenure ID, |
| let cur_burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(burn_db.conn()) | ||
| .expect("FATAL: failed to query sortition DB for canonical burn chain tip"); | ||
|
|
||
| if cur_burn_chain_tip.consensus_hash != block.header.consensus_hash { |
There was a problem hiding this comment.
You wouldn't want to compare to the block's consensus hash, because the sortition may have been empty and the current miner's tenure may still be ongoing. Would it make more sense to compare the sortition tips before and after building the block?
There was a problem hiding this comment.
I'm not sure yet how the nakamoto MinerThread should handle the sortition changing. My current plan is to treat it like a tenure change and do a thread stop and start. That way, the MinerThread really should stop mining if it detects a consensus hash change (which it will sort of need to do in any event: it will have to get a new tenure change tx), because each instantiation of a MinerThread will only ever be active for one bitcoin block.
| None | ||
| }; | ||
|
|
||
| parent_block_info.stacks_parent_header.microblock_tail = None; |
There was a problem hiding this comment.
Might also want to set the seq to 0
There was a problem hiding this comment.
The sequence number is part of that StacksMicroblockHeader struct, which is being set to None here, right?
| ) | ||
| .expect("BUG: failure processing network results"); | ||
|
|
||
| if net_receipts.num_new_blocks > 0 || net_receipts.num_new_confirmed_microblocks > 0 { |
There was a problem hiding this comment.
Does this also check Nakamoto blocks?
There was a problem hiding this comment.
This method is unchanged from the neon_node's process_net_result method.
| pox_watchdog: Option<PoxSyncWatchdog>, // can't be instantiated until .start() is called | ||
| is_miner: Option<bool>, // not known until .start() is called | ||
| burnchain: Option<Burnchain>, // not known until .start() is called | ||
| pox_watchdog_comms: PoxSyncWatchdogComms, |
There was a problem hiding this comment.
Right now, I'm just using it as a placeholder for an ibd global, I think as we move towards Argon, there will need to be a new struct for this, but I'd prefer to hold off on making a decision on that for now.
| @@ -63,6 +64,10 @@ pub struct Counters { | |||
| pub missed_tenures: RunLoopCounter, | |||
| pub missed_microblock_tenures: RunLoopCounter, | |||
| pub cancelled_commits: RunLoopCounter, | |||
|
|
|||
| pub naka_submitted_vrfs: RunLoopCounter, | |||
There was a problem hiding this comment.
PR is working for me to spin up a node to epoch2.5 with a fully functional pox-4. I've been unable to get it to boot into epoch3.0 (trying to replicate your integration test). It's likely just a block height config and pox cycle config issue on my end, but would be really helpful if:
- Default pox cycle config for neon works to boot epoch3.0 (maybe it already is, but not sure yet)
- While loading the config, if the given epoch block heights and pox config do not allow epoch3.0 to activate, then exit with error (e.g. invalid cycle boundaries)
kantai
force-pushed
the
feat/naka-neon
branch
from
756e498 to
aa5ca43
Compare
* Refactor some of the reused structs from `neon_node` * Fix a logic-bug in `nakamoto::coordinator`: the first prepare phase information will be a Epoch2x block, so the reward set calculation has to handle that. * Add `nakamoto_node` module based on `neon_node` * Add simple integration test for `nakamoto_node`
…ommits at tenure_id changes, cargo fmt-stacks
kantai
force-pushed
the
feat/naka-neon
branch
from
390dffc to
02c6457
Compare
…amoto_integrations test to bitcoin-tests group
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR implements a first version of a "nakamoto neon" node and operation mode.
As part of this implementation, this PR does several things:
neon_node(most notablyglobals).nakamoto::coordinator: the first prepare phase information will be a Epoch2x block, so the reward set calculation has to handle that.nakamoto_nodemodule based onneon_node. It uses a new nakamoto-specifixRelayerDirectiveenum.boot_nakamotomodule which wraps and orchestrates the 2.x <-> 3.x node handoff. It wraps aneonornakamotorunloop, and when the Epoch-3.0 transition is reached, stops theneonloop and starts thenakamotoloop.nakamoto_node: this integration test actually spins up a neon node to mine through 2.0-2.5, stack enough to active pox-4, then shuts down the neon node and spins up the nakamoto node. This is effectively a first test of node/miner operation during the transition from 2.x to 3.xThings that are still todo for this PR:
14fc7fe - Update: these are now all done.
Tests
Implementation
Block commits need to support RBF and check if the tip tenure ID changed. The reason for this is that miners should normally expect to RBF their block commit at least once:
(updated in 14fc7fe)
Upstream PRs
There are multiple PRs that will impact this one:
nakamoto_nodeso that it generates aTenureChangetx correctly.set-aggregate-public-keyprivate #4126 and Feat/mockamoto genesis block routine #4113 -- the aggregate public key setting should eventually impact thenakamoto_node. Right now, thenakamoto_nodeavoids the issue by explicitly passing its self-signing aggregate key to the block acceptance routine, but there probably needs to be a new issue for loading that aggregate key automatically (because such circumvention shouldn't be possible).