Skip to content
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

Secure cadence master update #2433

Merged
merged 61 commits into from
May 17, 2022

Conversation

m4ksio
Copy link
Contributor

@m4ksio m4ksio commented May 17, 2022

Merge master into secure cadence

zhiqiangxu and others added 30 commits December 10, 2021 20:34
Bump version because v1 is deprecated/sunset.
index ref_height->cluster_block_id on finalization
m4ksio and others added 15 commits May 12, 2022 11:50
[Execution State] Full search json dump
* update comments
* add benchmark for index lookup
The emulator repo implements Headers, and is imported by flow-go, so the
added method breaks test

Added as a follow-up for another issue: onflow/flow-go-internal#6244
add flaky skip reason so tests can run in cron job
2385: Robust Transaction Deduplication r=jordanschalm a=jordanschalm

This PR improves transaction de-duplication in collection nodes. It replaces a heuristic which de-duplicated based on the contents of a fixed number of ancestor collections, with an reference height index which enables de-duplicating transactions correctly in all cases including when the collection production rate significantly outpaces the consensus block rate. 

See also #2386 targeting the `v0.25` branch.

## Changes
* Add an index mapping reference block height to collection ID, for non-empty finalized collections
* Modify transaction inclusion validation when building and validating collections to take into account potentially conflicting collections at any point in the cluster state ancestry

Co-authored-by: Jordan Schalm <[email protected]>
@m4ksio m4ksio requested a review from turbolent May 17, 2022 00:18
@m4ksio m4ksio changed the base branch from master to feature/secure-cadence May 17, 2022 00:26
@github-actions
Copy link
Contributor

github-actions bot commented May 17, 2022

FVM Benchstat comparison

This branch with compared with the base branch onflow:master commit 7bea1c3

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

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

Collapsed results for better readability

old.txtnew.txt
time/opdelta
RuntimeTransaction/reference_tx-229.1ms ± 2%32.1ms ± 0%~(p=0.286 n=6+1)
RuntimeTransaction/convert_int_to_string-230.3ms ± 1%33.1ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-232.1ms ± 2%34.0ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_address-229.4ms ± 1%31.8ms ± 0%~(p=0.286 n=6+1)
RuntimeTransaction/get_public_account-232.8ms ± 3%35.9ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_balance-2543ms ± 3%571ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_available_balance-2436ms ± 1%472ms ± 0%~(p=0.286 n=6+1)
RuntimeTransaction/get_account_and_get_storage_used-243.7ms ± 3%46.3ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_storage_capacity-2400ms ± 2%443ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_vault-237.2ms ± 1%44.1ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_receiver-246.7ms ± 4%54.5ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/transfer_tokens-2174ms ± 1%203ms ± 0%~(p=0.286 n=6+1)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-236.7ms ± 2%39.2ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/load_and_save_long_string_on_signers_address-278.5ms ± 2%79.4ms ± 0%~(p=0.857 n=6+1)
RuntimeTransaction/create_new_account-21.10s ± 1%1.16s ± 0%~(p=0.286 n=6+1)
RuntimeTransaction/call_empty_contract_function-233.7ms ± 2%36.7ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/emit_event-247.2ms ± 4%50.8ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/borrow_array_from_storage-2112ms ± 3%109ms ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/copy_array_from_storage-2103ms ± 2%107ms ± 0%~(p=0.250 n=7+1)
 
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-2802 ± 0%802 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-22.40k ± 0%2.40k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-21.10k ± 0%1.10k ± 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-245.6k ± 0%46.7k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/convert_int_to_string-245.6k ± 0%46.7k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-245.6k ± 0%46.7k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_address-245.6k ± 0%46.7k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_public_account-245.6k ± 0%46.7k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_balance-216.7M ± 0%16.8M ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_available_balance-25.32M ± 0%5.33M ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_storage_used-248.4k ± 0%49.5k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_account_and_get_storage_capacity-25.32M ± 0%5.32M ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_vault-246.8k ± 0%48.0k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/get_signer_receiver-247.2k ± 0%48.4k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/transfer_tokens-248.0k ± 0%49.1k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-249.0k ± 0%50.2k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/load_and_save_long_string_on_signers_address-254.0k ± 0%55.2k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/create_new_account-28.44M ± 0%8.47M ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/call_empty_contract_function-245.8k ± 0%46.9k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/emit_event-245.8k ± 0%46.9k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/borrow_array_from_storage-252.0k ± 0%53.1k ± 0%~(p=0.250 n=7+1)
RuntimeTransaction/copy_array_from_storage-252.0k ± 0%53.1k ± 0%~(p=0.250 n=7+1)
 

@codecov-commenter
Copy link

Codecov Report

Merging #2433 (b6dcac4) into feature/secure-cadence (7a87823) will increase coverage by 0.67%.
The diff coverage is 78.11%.

@@                    Coverage Diff                     @@
##           feature/secure-cadence    #2433      +/-   ##
==========================================================
+ Coverage                   56.28%   56.95%   +0.67%     
==========================================================
  Files                         653      653              
  Lines                       40114    60019   +19905     
==========================================================
+ Hits                        22577    34183   +11606     
- Misses                      14658    22911    +8253     
- Partials                     2879     2925      +46     
Flag Coverage Δ
unittests 56.95% <78.11%> (+0.67%) ⬆️

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

Impacted Files Coverage Δ
cmd/bootstrap/run/qc.go 37.06% <0.00%> (-3.21%) ⬇️
model/flow/address.go 79.83% <ø> (-1.42%) ⬇️
model/flow/constants.go 75.00% <ø> (+15.00%) ⬆️
module/builder/collection/tx_lookup.go 100.00% <ø> (ø)
storage/badger/headers.go 41.44% <0.00%> (-2.90%) ⬇️
storage/badger/operation/prefix.go 79.31% <ø> (+3.31%) ⬆️
model/flow/cluster.go 54.28% <25.00%> (+1.78%) ⬆️
module/finalizer/collection/finalizer.go 73.40% <52.94%> (-2.87%) ⬇️
state/cluster/badger/mutator.go 74.16% <73.00%> (-1.37%) ⬇️
storage/badger/operation/cluster.go 87.09% <80.95%> (+15.66%) ⬆️
... and 659 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a87823...b6dcac4. Read the comment docs.

@m4ksio m4ksio requested a review from j1010001 May 17, 2022 02:22
@m4ksio m4ksio merged commit 47da842 into feature/secure-cadence May 17, 2022
@m4ksio m4ksio deleted the m4ksio/secure-cadence-master-update branch May 17, 2022 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.