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

tweak: Standardize resource assertion errors #1975

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Oct 25, 2024

Summary

Standardizes resource assertion errors across the old assertions and the new assertions and makes some fixes/tweaks:

  • Added the same error message to ASSERT_WORKTOP_CONTAINS and its cousins, as requested by Les.
  • Added a resource_address to contextualise constraint failure messages.
  • Removed nesting / duplication in GeneralResourceAssertionError to simplify the errors a little and make it easier to match on in a client.
  • Removed unbounded non fungible id lists from the error messages.
  • Removed swap_remove during validation of constraints to ensure the selected error is intuitive (i.e. the first erroring id is returned).

And a few other misc refactors as part of fixing the tests. (e.g. adding a structural PartialEq to Decimal via ensuring these traits are implemented in macros for the Ixxx number types -- this enables it to be used in patterns, e.g. in matches! macros in tests).

Testing

Existing tests pass (with minor tweaks)

@dhedey dhedey force-pushed the tweak/standardize-assertion-errors branch from 950d185 to 9f2c001 Compare October 25, 2024 03:04
Copy link

github-actions bot commented Oct 25, 2024

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

Copy link

github-actions bot commented Oct 25, 2024

Benchmark for cecdf21

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 44.3±0.10ms 44.4±0.14ms +0.23%
costing::decode_encoded_i8_array_to_manifest_raw_value 19.3±0.02ms 19.3±0.02ms 0.00%
costing::decode_encoded_i8_array_to_manifest_value 41.8±0.13ms 41.6±0.07ms -0.48%
costing::decode_encoded_tuple_array_to_manifest_raw_value 70.3±0.04ms 70.4±0.16ms +0.14%
costing::decode_encoded_tuple_array_to_manifest_value 109.0±0.73ms 98.8±0.21ms -9.36%
costing::decode_encoded_u8_array_to_manifest_raw_value 31.9±0.13µs 25.7±0.48µs -19.44%
costing::decode_encoded_u8_array_to_manifest_value 41.6±0.19ms 41.6±0.20ms 0.00%
costing::decode_rpd_to_manifest_raw_value 14.5±0.05µs 14.9±0.03µs +2.76%
costing::decode_rpd_to_manifest_value 10.6±0.06µs 11.1±0.09µs +4.72%
costing::deserialize_wasm 1199.7±2.96µs 1229.9±6.33µs +2.52%
costing::execute_transaction_creating_big_vec_substates 714.2±27.30ms 685.0±11.36ms -4.09%
costing::execute_transaction_reading_big_vec_substates 617.2±14.01ms 599.4±2.83ms -2.88%
costing::instantiate_flash_loan 1004.7±1309.65µs 898.5±376.46µs -10.57%
costing::instantiate_radiswap 947.1±723.87µs 985.5±841.67µs +4.05%
costing::spin_loop 18.9±0.04ms 18.5±0.02ms -2.12%
costing::spin_loop_v2 2.6±0.00s 2.6±0.02s 0.00%
costing::spin_loop_v3 607.0±4.76ms 624.5±3.76ms +2.88%
costing::validate_sbor_payload 29.6±0.08µs 29.4±0.13µs -0.68%
costing::validate_sbor_payload_bytes 272.5±3.43ns 245.2±2.78ns -10.02%
costing::validate_secp256k1 76.7±0.07µs 76.7±0.22µs 0.00%
costing::validate_wasm 33.7±0.06ms 34.0±0.06ms +0.89%
decimal::add/0 8.4±0.01ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.9±0.03ns 9.8±0.01ns -1.01%
decimal::add/wasmi 217.2±0.25ns 221.4±0.86ns +1.93%
decimal::add/wasmi-call-native 2.2±0.00µs 2.1±0.01µs -4.55%
decimal::div/0 164.3±0.20ns 169.1±0.12ns +2.92%
decimal::from_string/0 163.0±0.44ns 156.9±0.17ns -3.74%
decimal::mul/0 126.0±0.24ns 128.4±0.11ns +1.90%
decimal::mul/rust-native 123.0±0.19ns 127.3±0.04ns +3.50%
decimal::mul/wasmi 11.1±0.06µs 11.5±0.12µs +3.60%
decimal::mul/wasmi-call-native 2.2±0.01µs 2.3±0.00µs +4.55%
decimal::pow/0 574.5±0.50ns 593.0±0.21ns +3.22%
decimal::pow/rust-native 575.0±0.92ns 588.5±0.87ns +2.35%
decimal::pow/wasmi 57.9±0.10µs 60.5±1.55µs +4.49%
decimal::pow/wasmi-call-native 3.2±0.01µs 3.2±0.01µs 0.00%
decimal::root/0 8.3±0.01µs 8.0±0.00µs -3.61%
decimal::sub/0 8.2±0.01ns 8.3±0.01ns +1.22%
decimal::to_string/0 438.6±0.44ns 440.7±1.83ns +0.48%
large_transaction_processing::prepare 2.4±0.00ms 2.5±0.00ms +4.17%
large_transaction_processing::prepare_and_decompile 6.5±0.01ms 6.3±0.01ms -3.08%
large_transaction_processing::prepare_and_decompile_and_recompile 25.8±1.29ms 31.8±0.11ms +23.26%
precise_decimal::add/0 8.8±0.02ns 9.0±0.02ns +2.27%
precise_decimal::add/rust-native 11.0±0.11ns 10.8±0.37ns -1.82%
precise_decimal::add/wasmi 273.7±1.06ns 278.9±0.66ns +1.90%
precise_decimal::add/wasmi-call-native 2.8±0.00µs 2.9±0.01µs +3.57%
precise_decimal::div/0 282.3±0.99ns 303.7±2.15ns +7.58%
precise_decimal::from_string/0 202.6±0.53ns 201.7±0.17ns -0.44%
precise_decimal::mul/0 327.6±3.83ns 334.2±0.29ns +2.01%
precise_decimal::mul/rust-native 278.4±1.59ns 286.6±0.81ns +2.95%
precise_decimal::mul/wasmi 34.3±0.08µs 34.7±0.17µs +1.17%
precise_decimal::mul/wasmi-call-native 3.1±0.01µs 3.2±0.01µs +3.23%
precise_decimal::pow/0 1701.3±3.35ns 1723.5±1.82ns +1.30%
precise_decimal::pow/rust-native 1348.6±6.61ns 1358.2±1.24ns +0.71%
precise_decimal::pow/wasmi 159.7±0.72µs 164.7±1.31µs +3.13%
precise_decimal::pow/wasmi-call-native 5.4±0.04µs 5.4±0.01µs 0.00%
precise_decimal::root/0 58.2±0.04µs 58.6±0.03µs +0.69%
precise_decimal::sub/0 9.0±0.11ns 9.2±0.04ns +2.22%
precise_decimal::to_string/0 696.4±0.97ns 696.0±0.98ns -0.06%
schema::validate_payload 383.2±0.80µs 395.2±1.35µs +3.13%
transaction::radiswap 4.7±0.03ms 4.8±0.03ms +2.13%
transaction::transfer 1819.7±2.87µs 1844.1±11.97µs +1.34%
transaction_validation::validate_manifest 43.1±0.03µs 43.3±0.13µs +0.46%
transaction_validation::verify_bls_2KB 1088.1±19.65µs 1012.5±16.20µs -6.95%
transaction_validation::verify_bls_32B 1017.9±19.26µs 1038.0±31.06µs +1.97%
transaction_validation::verify_ecdsa 74.6±0.06µs 74.5±0.10µs -0.13%
transaction_validation::verify_ed25519 44.6±0.06µs 44.4±0.07µs -0.45%

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.

1 participant