flow-go icon indicating copy to clipboard operation
flow-go copied to clipboard

Update HotStuff Finalization Logic

Open jordanschalm opened this issue 2 years ago • 3 comments

This PR updates Forks to implement 2-chain finalization as defined in Jolteon (and later adopted in DiemBFT v4).

  • Updates error handling and documentation in LevelledForest
  • Updates error handling and documentation in Forks
  • Updates finalization logic in Forks
    • A block can be finalized when it has a direct 1-chain plus an indirect 1-chain
    • Removes lockedBlock book-keeping - this is no longer relevant

See https://github.com/dapperlabs/flow-go/issues/6191 for additional detail.

jordanschalm avatar Aug 10 '22 01:08 jordanschalm

Can we update one test as part of this PR? It assumes that we have a 3-chain.

durkmurder avatar Aug 11 '22 16:08 durkmurder

FVM Benchstat comparison

This branch with compared with the base branch onflow:feature/active-pacemaker commit c5443e73fa94b81fa51a64d79bec3ac8e84529e1

The command (for i in {1..N}; do go test ./fvm --bench . --tags relic -shuffle=on --benchmem --run ^$; done) was used.

Bench tests were run a total of 10 times on each branch.

Collapsed results for better readability

old.txtnew.txt
time/opdelta
RuntimeTransaction/reference_tx-226.5ms ± 7%26.1ms ± 1%~(p=0.182 n=10+9)
RuntimeTransaction/convert_int_to_string-227.6ms ± 2%27.6ms ± 2%~(p=0.633 n=8+10)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-229.1ms ± 3%28.9ms ± 2%~(p=0.387 n=9+9)
RuntimeTransaction/get_signer_address-228.1ms ±14%27.7ms ±15%~(p=0.436 n=10+10)
RuntimeTransaction/get_public_account-229.2ms ± 1%29.3ms ± 3%~(p=0.436 n=9+9)
RuntimeTransaction/get_account_and_get_balance-2494ms ±19%473ms ± 2%~(p=0.400 n=10+9)
RuntimeTransaction/get_account_and_get_available_balance-2385ms ± 2%382ms ± 1%~(p=0.247 n=10+10)
RuntimeTransaction/get_account_and_get_storage_used-233.8ms ± 1%33.8ms ± 1%~(p=0.684 n=10+10)
RuntimeTransaction/get_account_and_get_storage_capacity-2356ms ± 2%354ms ± 1%~(p=0.400 n=10+9)
RuntimeTransaction/get_signer_vault-233.5ms ± 2%33.3ms ± 2%~(p=0.315 n=10+10)
RuntimeTransaction/get_signer_receiver-242.8ms ± 2%42.8ms ± 1%~(p=0.497 n=9+10)
RuntimeTransaction/transfer_tokens-2173ms ± 1%173ms ± 1%~(p=0.481 n=9+8)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-233.1ms ± 2%32.8ms ± 1%~(p=0.173 n=10+8)
RuntimeTransaction/load_and_save_long_string_on_signers_address-273.0ms ± 3%72.4ms ± 2%~(p=0.095 n=10+9)
RuntimeTransaction/create_new_account-21.04s ± 2%1.03s ± 1%~(p=0.278 n=9+10)
RuntimeTransaction/call_empty_contract_function-228.9ms ± 3%28.7ms ± 1%~(p=0.133 n=10+9)
RuntimeTransaction/emit_event-242.1ms ± 2%42.0ms ± 2%~(p=0.549 n=10+9)
RuntimeTransaction/borrow_array_from_storage-2110ms ± 2%109ms ± 1%~(p=0.353 n=10+10)
RuntimeNFTBatchTransfer-2111ms ± 2%110ms ± 2%~(p=0.182 n=10+9)
RuntimeTransaction/copy_array_from_storage-2110ms ± 3%108ms ± 2%−1.20%(p=0.029 n=10+10)
 
computationdelta
RuntimeTransaction/reference_tx-2202 ± 0%202 ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2502 ± 0%502 ± 0%~(all equal)
RuntimeTransaction/get_signer_address-2302 ± 0%302 ± 0%~(all equal)
RuntimeTransaction/get_public_account-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-21.00k ± 0%1.00k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-22.50k ± 0%2.50k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-21.30k ± 0%1.30k ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-23.50k ± 0%3.50k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/create_new_account-2202 ± 0%202 ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/emit_event-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-22.60k ± 0%2.60k ± 0%~(all equal)
RuntimeTransaction/copy_array_from_storage-22.60k ± 0%2.60k ± 0%~(all equal)
 
interactionsdelta
RuntimeTransaction/reference_tx-247.4k ± 0%47.4k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-247.4k ± 0%47.4k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-247.4k ± 0%47.4k ± 0%~(all equal)
RuntimeTransaction/get_signer_address-247.4k ± 0%47.4k ± 0%~(all equal)
RuntimeTransaction/get_public_account-247.4k ± 0%47.4k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-216.7M ± 0%16.7M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-25.30M ± 0%5.30M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-250.2k ± 0%50.2k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-25.30M ± 0%5.30M ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-248.6k ± 0%48.6k ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-249.0k ± 0%49.0k ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-249.8k ± 0%49.8k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-250.9k ± 0%50.9k ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-255.9k ± 0%55.9k ± 0%~(all equal)
RuntimeTransaction/create_new_account-28.43M ± 0%8.43M ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-247.6k ± 0%47.6k ± 0%~(all equal)
RuntimeTransaction/emit_event-247.6k ± 0%47.6k ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-253.8k ± 0%53.8k ± 0%~(all equal)
RuntimeTransaction/copy_array_from_storage-253.8k ± 0%53.8k ± 0%~(all equal)
 
alloc/opdelta
RuntimeTransaction/transfer_tokens-250.4MB ± 0%50.5MB ± 0%+0.27%(p=0.006 n=10+8)
RuntimeTransaction/get_signer_vault-211.3MB ± 0%11.3MB ± 0%+0.24%(p=0.004 n=9+10)
RuntimeTransaction/reference_tx-29.20MB ± 1%9.20MB ± 1%~(p=0.912 n=10+10)
RuntimeTransaction/convert_int_to_string-29.68MB ± 1%9.66MB ± 0%~(p=0.278 n=10+9)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-210.2MB ± 1%10.2MB ± 1%~(p=0.218 n=10+10)
RuntimeTransaction/get_signer_address-29.42MB ± 1%9.45MB ± 1%~(p=0.079 n=9+10)
RuntimeTransaction/get_public_account-210.9MB ± 0%10.9MB ± 1%~(p=0.661 n=9+10)
RuntimeTransaction/get_account_and_get_available_balance-2147MB ± 0%147MB ± 0%~(p=0.218 n=10+10)
RuntimeTransaction/get_account_and_get_storage_used-212.2MB ± 0%12.3MB ± 0%~(p=0.387 n=9+9)
RuntimeTransaction/get_account_and_get_storage_capacity-2142MB ± 0%142MB ± 0%~(p=0.315 n=9+10)
RuntimeTransaction/get_signer_receiver-215.2MB ± 1%15.2MB ± 1%~(p=0.436 n=10+10)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-210.8MB ± 0%10.8MB ± 1%~(p=0.604 n=9+10)
RuntimeTransaction/load_and_save_long_string_on_signers_address-225.5MB ± 0%25.5MB ± 0%~(p=0.481 n=10+10)
RuntimeTransaction/create_new_account-2284MB ± 0%284MB ± 0%~(p=0.065 n=9+10)
RuntimeTransaction/call_empty_contract_function-210.0MB ± 1%10.0MB ± 0%~(p=0.400 n=10+9)
RuntimeTransaction/emit_event-214.3MB ± 1%14.3MB ± 1%~(p=0.579 n=10+10)
RuntimeTransaction/borrow_array_from_storage-235.5MB ± 0%35.5MB ± 0%~(p=0.050 n=9+9)
RuntimeTransaction/copy_array_from_storage-246.7MB ± 0%46.7MB ± 0%~(p=0.739 n=10+10)
RuntimeNFTBatchTransfer-228.0MB ± 0%28.0MB ± 0%~(p=0.393 n=10+10)
RuntimeTransaction/get_account_and_get_balance-2197MB ± 0%197MB ± 0%−0.01%(p=0.034 n=8+10)
 
allocs/opdelta
RuntimeTransaction/get_signer_vault-2200k ± 0%200k ± 0%+0.01%(p=0.012 n=10+9)
RuntimeTransaction/reference_tx-2147k ± 0%147k ± 0%~(p=0.468 n=10+10)
RuntimeTransaction/convert_int_to_string-2162k ± 0%162k ± 0%~(p=0.068 n=9+9)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2176k ± 0%176k ± 0%~(p=0.825 n=9+10)
RuntimeTransaction/get_signer_address-2152k ± 0%152k ± 0%~(p=0.809 n=10+10)
RuntimeTransaction/get_public_account-2184k ± 0%184k ± 0%~(p=0.068 n=10+9)
RuntimeTransaction/get_account_and_get_balance-23.27M ± 0%3.27M ± 0%~(p=0.656 n=8+9)
RuntimeTransaction/get_account_and_get_available_balance-22.64M ± 0%2.64M ± 0%~(p=0.579 n=10+10)
RuntimeTransaction/get_account_and_get_storage_used-2205k ± 0%205k ± 0%~(p=0.897 n=10+10)
RuntimeTransaction/get_account_and_get_storage_capacity-22.49M ± 0%2.49M ± 0%~(p=0.926 n=10+10)
RuntimeTransaction/get_signer_receiver-2289k ± 0%289k ± 0%~(p=0.702 n=9+10)
RuntimeTransaction/transfer_tokens-21.06M ± 0%1.06M ± 0%~(p=0.123 n=10+10)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2198k ± 0%198k ± 0%~(p=0.218 n=10+9)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2297k ± 0%297k ± 0%~(p=0.362 n=10+10)
RuntimeTransaction/create_new_account-24.47M ± 0%4.47M ± 0%~(p=0.093 n=10+10)
RuntimeTransaction/call_empty_contract_function-2164k ± 0%164k ± 0%~(p=0.748 n=10+10)
RuntimeTransaction/emit_event-2211k ± 0%211k ± 0%~(p=0.203 n=10+9)
RuntimeTransaction/borrow_array_from_storage-2437k ± 0%437k ± 0%~(p=0.514 n=10+10)
RuntimeTransaction/copy_array_from_storage-2393k ± 0%393k ± 0%~(p=0.562 n=9+10)
RuntimeNFTBatchTransfer-2382k ± 0%382k ± 0%~(p=0.494 n=10+10)
 

github-actions[bot] avatar Aug 12 '22 01:08 github-actions[bot]

Codecov Report

Merging #2954 (af7e70a) into feature/active-pacemaker (382a3fd) will decrease coverage by 0.03%. The diff coverage is 64.88%.

:exclamation: Current head af7e70a differs from pull request most recent head 4a2d1e9. Consider uploading reports for the commit 4a2d1e9 to get more accurate results

@@                     Coverage Diff                      @@
##           feature/active-pacemaker    #2954      +/-   ##
============================================================
- Coverage                     56.74%   56.71%   -0.04%     
============================================================
  Files                           696      695       -1     
  Lines                         65006    64890     -116     
============================================================
- Hits                          36887    36801      -86     
+ Misses                        25021    24995      -26     
+ Partials                       3098     3094       -4     
Flag Coverage Δ
unittests 56.71% <64.88%> (-0.04%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
consensus/hotstuff/forks/blockcontainer.go 100.00% <ø> (ø)
module/forest/vertex.go 87.50% <0.00%> (-12.50%) :arrow_down:
consensus/hotstuff/forks/forks.go 77.23% <63.85%> (+6.75%) :arrow_up:
module/forest/leveled_forest.go 83.23% <71.11%> (ø)
fvm/handler/contract.go 88.59% <0.00%> (-1.35%) :arrow_down:
consensus/hotstuff/forks/block_builder_test.go
utils/debug/profiler.go 84.37% <0.00%> (+2.08%) :arrow_up:
...s/hotstuff/timeoutaggregator/timeout_collectors.go 82.08% <0.00%> (+5.97%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 12 '22 18:08 codecov-commenter

bors merge

jordanschalm avatar Aug 23 '22 19:08 jordanschalm

Build succeeded:

bors[bot] avatar Aug 23 '22 20:08 bors[bot]