fix: the race condition in compact

[garyyu]

In #2903, I thought the race condition fixing is not urgent based on my silly assumption of its low possibility:

There could be a race condition here if a new block coming in and seize the locking among these rebuild, even in a very low possibility.

But actually I find it's NOT, based on a bug I find today which has a "block_height": null for /v1/txhashset/outputs node API query result.

$ curl -0 -XGET  http://127.0.0.1:13413/v1/txhashset/outputs?start_index=320114\&max=1

{
  "highest_index": 321346,
  "last_retrieved_index": 320114,
  "outputs": [
    {
      "output_type": "Coinbase",
      "commit": "08511d10773136727b13f6bcd21dc62d066425439bdcb708d62456fc8b2f307691",
      "spent": true,
      "proof": "6180cee3 ... ...",
      "proof_hash": "9551a07966bc438f106c982f7eb734b928632662189ebe1d271616f592f697da",
      "block_height": null,
      "merkle_proof": null,
      "mmr_index": 0
    }
  ]
}

Analysis of the following log:

20190831 22:50:12.752 DEBUG grin_chain::chain - compact: head: 228987, tail: 216353, diff: 12634, horizon: 10080
20190831 22:50:12.752 DEBUG grin_chain::txhashset::txhashset - txhashset: starting compaction...
...
20190831 22:50:19.443 DEBUG grin_servers::common::adapters - Received compact_block 057049f212c5 at 228988 from 35.247.29.132:3414 [out/kern/kern_ids: 1/1/0] going to process.
...
20190831 22:51:25.050 DEBUG grin_chain::txhashset::txhashset - txhashset: ... compaction finished
20190831 22:51:25.388 DEBUG grin_chain::txhashset::txhashset - txhashset: rebuild_index: 117974 UTXOs, took 0s
...
20190831 22:51:26.216 DEBUG grin_chain::pipe - pipe: process_block 057049f212c5 at 228988 [in/out/kern: 0/1/1]
20190831 22:51:26.218 DEBUG grin_chain::pipe - pipe: header_head updated to 057049f212c5 at 228988
...
20190831 22:51:26.261 DEBUG grin_chain::chain - rebuild_height_for_pos: rebuilding 117974 output_pos's height...

It shows the gap time between compact starting and rebuild_height_for_pos could be very big (
1 minutes 14 seconds at above log). During this big gap, there could be some new blocks queued there to be processed. And once the new blocks processed before calling this rebuild_height_for_pos, the problem will happen: rebuild_height_for_pos will lose the indexes of the new processed blocks, because a simple batch.clear_output_pos_height() there in rebuild_height_for_pos.

The fix solution is to replace the rebuild_index with the new rebuild_height_for_pos, to avoid the 2-stages rebuilding.

Note:

• txhashset_write is still using the 2-stages rebuilding. But it's not a problem like in the compact, because the txhashset_write only can be called on the state sync stage, which forbid the block processing.
• txhashset_write refactoring (to avoid multiple rebuilding split into multiple locking) can be considered after the related PR Split header MMR (and sync MMR) out from txhashset #3004 merged, to also remove this 2-stages rebuilding. (but that's a pure improvement, not a bug fix like this PR).

[antiochp]

Looks good to me.