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

Tests/transaction validation refactor #2012

Merged
merged 20 commits into from
Nov 26, 2024

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Nov 22, 2024

Summary

Splits the TransactionValidator across multiple files, and adds neatenings, for the following benefits:

  • Separate files make it easier to follow and add tests without navigating around a single massive file
  • Improved abstractions (see e.g. IntentStructure, IntentTreeStructure, SignedIntentTreeStructure)
    make the logic more generic and easier to follow; and makes it harder to miss validations.

Testing

Existing tests have moved around to the relevant files, but none have been removed.

@dhedey dhedey changed the base branch from main to release/cuttlefish November 22, 2024 15:40
Copy link

github-actions bot commented Nov 22, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:fb65d359d3

Copy link

github-actions bot commented Nov 22, 2024

Benchmark for fb65d35

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 44.7±0.09ms 44.6±0.07ms -0.22%
costing::decode_encoded_i8_array_to_manifest_raw_value 19.3±0.00ms 19.3±0.02ms 0.00%
costing::decode_encoded_i8_array_to_manifest_value 41.6±0.03ms 42.1±0.06ms +1.20%
costing::decode_encoded_tuple_array_to_manifest_raw_value 76.0±0.06ms 72.1±0.08ms -5.13%
costing::decode_encoded_tuple_array_to_manifest_value 98.0±0.14ms 98.1±0.13ms +0.10%
costing::decode_encoded_u8_array_to_manifest_raw_value 32.1±0.07µs 25.7±0.10µs -19.94%
costing::decode_encoded_u8_array_to_manifest_value 41.5±0.03ms 42.0±0.03ms +1.20%
costing::decode_rpd_to_manifest_raw_value 14.6±0.03µs 14.6±0.05µs 0.00%
costing::decode_rpd_to_manifest_value 11.0±0.05µs 11.0±0.03µs 0.00%
costing::deserialize_wasm 1230.3±8.07µs 1217.4±3.30µs -1.05%
costing::execute_transaction_creating_big_vec_substates 683.0±4.36ms 685.7±8.54ms +0.40%
costing::execute_transaction_reading_big_vec_substates 585.6±2.50ms 625.0±0.27ms +6.73%
costing::instantiate_flash_loan 814.9±121.15µs 936.4±608.97µs +14.91%
costing::instantiate_radiswap 886.2±352.31µs 945.2±833.71µs +6.66%
costing::scrypto_malloc 672.1±0.94ms 674.4±1.71ms +0.34%
costing::scrypto_sbor_decode 670.4±1.40ms 667.5±1.22ms -0.43%
costing::scrypto_sha256 594.2±3.79ms 592.9±2.08ms -0.22%
costing::spin_loop_v1 518.6±1.04ms 519.7±0.98ms +0.21%
costing::spin_loop_v2 596.2±0.74ms 598.5±2.31ms +0.39%
costing::validate_sbor_payload 29.3±0.05µs 29.3±0.06µs 0.00%
costing::validate_sbor_payload_bytes 263.8±2.36ns 257.6±2.32ns -2.35%
costing::validate_secp256k1 76.6±0.59µs 76.5±0.06µs -0.13%
costing::validate_wasm 34.5±0.04ms 33.6±0.04ms -2.61%
decimal::add/0 8.4±0.00ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.8±0.00ns 9.8±0.00ns 0.00%
decimal::add/wasmi 317.8±1.30ns 317.5±1.06ns -0.09%
decimal::add/wasmi-call-native 3.1±0.01µs 3.2±0.00µs +3.23%
decimal::div/0 173.0±0.12ns 167.1±0.06ns -3.41%
decimal::from_string/0 156.2±0.11ns 156.8±0.09ns +0.38%
decimal::mul/0 127.9±0.15ns 127.5±0.03ns -0.31%
decimal::mul/rust-native 128.0±0.17ns 124.8±0.14ns -2.50%
decimal::mul/wasmi 19.8±0.08µs 19.6±0.03µs -1.01%
decimal::mul/wasmi-call-native 3.2±0.01µs 3.3±0.00µs +3.12%
decimal::pow/0 592.7±0.40ns 593.4±0.34ns +0.12%
decimal::pow/rust-native 590.2±0.28ns 586.8±0.20ns -0.58%
decimal::pow/wasmi 99.0±0.13µs 92.7±0.15µs -6.36%
decimal::pow/wasmi-call-native 5.1±0.01µs 5.0±0.02µs -1.96%
decimal::root/0 8.4±0.01µs 8.2±0.01µs -2.38%
decimal::sub/0 8.2±0.00ns 8.2±0.00ns 0.00%
decimal::to_string/0 444.5±1.55ns 445.4±1.74ns +0.20%
large_transaction_processing::prepare 2.5±0.01ms 2.6±0.00ms +4.00%
large_transaction_processing::prepare_and_decompile 6.3±0.01ms 6.2±0.01ms -1.59%
large_transaction_processing::prepare_and_decompile_and_recompile 29.7±2.59ms 24.8±0.06ms -16.50%
metadata_validation::validate_urls 5.1±0.01µs 4.7±0.01µs -7.84%
precise_decimal::add/0 8.9±0.00ns 8.7±0.07ns -2.25%
precise_decimal::add/rust-native 10.6±0.01ns 10.7±0.02ns +0.94%
precise_decimal::add/wasmi 455.6±1.51ns 457.7±1.84ns +0.46%
precise_decimal::add/wasmi-call-native 4.1±0.01µs 4.0±0.01µs -2.44%
precise_decimal::div/0 316.0±1.40ns 305.6±4.28ns -3.29%
precise_decimal::from_string/0 201.4±0.13ns 201.3±0.30ns -0.05%
precise_decimal::mul/0 331.8±1.16ns 337.2±1.81ns +1.63%
precise_decimal::mul/rust-native 284.2±0.49ns 283.2±0.28ns -0.35%
precise_decimal::mul/wasmi 47.5±0.17µs 47.6±0.16µs +0.21%
precise_decimal::mul/wasmi-call-native 4.5±0.00µs 4.4±0.00µs -2.22%
precise_decimal::pow/0 1774.5±3.07ns 1771.0±1.04ns -0.20%
precise_decimal::pow/rust-native 1359.0±0.91ns 1352.1±0.94ns -0.51%
precise_decimal::pow/wasmi 228.0±0.60µs 232.7±1.52µs +2.06%
precise_decimal::pow/wasmi-call-native 7.7±0.02µs 7.7±0.01µs 0.00%
precise_decimal::root/0 58.1±0.04µs 58.5±0.04µs +0.69%
precise_decimal::sub/0 9.2±0.01ns 9.0±0.15ns -2.17%
precise_decimal::to_string/0 694.7±0.68ns 696.3±1.07ns +0.23%
schema::validate_payload 385.2±0.40µs 385.9±0.47µs +0.18%
transaction::radiswap 4.9±0.02ms 5.0±0.03ms +2.04%
transaction::transfer 1861.3±4.56µs 1829.0±2.36µs -1.74%
transaction_validation::validate_manifest 43.1±0.13µs 42.9±0.04µs -0.46%
transaction_validation::verify_bls_2KB 964.8±7.81µs 967.4±10.14µs +0.27%
transaction_validation::verify_bls_32B 968.3±10.85µs 961.9±9.99µs -0.66%
transaction_validation::verify_ecdsa 74.4±0.08µs 74.5±0.06µs +0.13%
transaction_validation::verify_ed25519 42.2±0.07µs 44.4±0.04µs +5.21%

iamyulong and others added 3 commits November 26, 2024 11:58
…dation-tests

Add further transaction validation tests
tests: Added tests for each `SubintentStructureError`
@iamyulong iamyulong merged commit 1b22e3e into release/cuttlefish Nov 26, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants