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.

Sign up for GitHub

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

Jump to bottom

Use Dynamic*Address types in the ManifestInput models #1992

Merged
merged 7 commits into from
Nov 1, 2024
Merged

Use Dynamic*Address types in the ManifestInput models #1992

merged 7 commits into from
Nov 1, 2024

Conversation

0xOmarA
Copy link
Member

@0xOmarA 0xOmarA commented Oct 31, 2024

Summary

This PR changes the address types used in the ManifestInput models to use dynamic addresses. This fixes an issue that we found in the static analyzer (which was propagated in the toolkit) where add_authorized_depositor with a NamedAddress would fail when we attempt to convert it into a typed invocation.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 31, 2024 •
edited
Loading

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 31, 2024 •
edited
Loading

Benchmark for 568c5c4

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 44.7±0.08ms 45.1±0.09ms +0.89%
costing::decode_encoded_i8_array_to_manifest_raw_value 19.3±0.01ms 19.6±0.02ms +1.55%
costing::decode_encoded_i8_array_to_manifest_value 42.2±0.06ms 41.4±0.06ms -1.90%
costing::decode_encoded_tuple_array_to_manifest_raw_value 72.1±0.05ms 63.3±0.11ms -12.21%
costing::decode_encoded_tuple_array_to_manifest_value 98.4±0.11ms 98.5±0.15ms +0.10%
costing::decode_encoded_u8_array_to_manifest_raw_value 31.6±0.08µs 25.9±0.03µs -18.04%
costing::decode_encoded_u8_array_to_manifest_value 42.1±0.06ms 41.5±0.05ms -1.43%
costing::decode_rpd_to_manifest_raw_value 14.5±0.04µs 12.5±0.02µs -13.79%
costing::decode_rpd_to_manifest_value 11.2±0.01µs 11.1±0.03µs -0.89%
costing::deserialize_wasm 1235.1±3.43µs 1214.6±3.59µs -1.66%
costing::execute_transaction_creating_big_vec_substates 698.4±10.10ms 694.3±9.89ms -0.59%
costing::execute_transaction_reading_big_vec_substates 590.8±1.27ms 598.7±1.63ms +1.34%
costing::instantiate_flash_loan 1004.0±1351.43µs 1005.3±1353.82µs +0.13%
costing::instantiate_radiswap 952.3±903.50µs 845.2±200.67µs -11.25%
costing::scrypto_malloc 494.6±0.56ms 485.9±0.72ms -1.76%
costing::scrypto_sbor_decode 519.1±1.22ms 526.6±1.70ms +1.44%
costing::scrypto_sha256 429.4±0.85ms 437.3±0.67ms +1.84%
costing::spin_loop_v1 433.2±0.84ms 436.1±2.39ms +0.67%
costing::spin_loop_v2 539.3±0.94ms 545.1±5.01ms +1.08%
costing::validate_sbor_payload 29.5±0.06µs 30.4±0.07µs +3.05%
costing::validate_sbor_payload_bytes 276.1±3.64ns 244.4±0.51ns -11.48%
costing::validate_secp256k1 76.6±0.04µs 76.5±0.08µs -0.13%
costing::validate_wasm 33.9±0.03ms 33.5±0.03ms -1.18%
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 216.2±0.17ns 225.8±0.40ns +4.44%
decimal::add/wasmi-call-native 2.1±0.00µs 2.1±0.01µs 0.00%
decimal::div/0 168.8±0.16ns 169.0±0.22ns +0.12%
decimal::from_string/0 155.6±0.18ns 155.8±0.11ns +0.13%
decimal::mul/0 129.6±0.13ns 129.4±0.09ns -0.15%
decimal::mul/rust-native 127.8±0.04ns 127.5±0.20ns -0.23%
decimal::mul/wasmi 11.5±0.08µs 11.5±0.05µs 0.00%
decimal::mul/wasmi-call-native 2.2±0.01µs 2.2±0.01µs 0.00%
decimal::pow/0 594.7±0.37ns 594.2±0.21ns -0.08%
decimal::pow/rust-native 590.7±0.24ns 589.3±0.33ns -0.24%
decimal::pow/wasmi 58.4±0.14µs 60.2±0.46µs +3.08%
decimal::pow/wasmi-call-native 3.2±0.00µs 3.2±0.00µs 0.00%
decimal::root/0 8.1±0.01µs 8.3±0.01µs +2.47%
decimal::sub/0 8.3±0.01ns 8.3±0.00ns 0.00%
decimal::to_string/0 443.1±1.01ns 439.4±0.16ns -0.84%
large_transaction_processing::prepare 2.6±0.00ms 2.5±0.00ms -3.85%
large_transaction_processing::prepare_and_decompile 6.2±0.01ms 6.2±0.03ms 0.00%
large_transaction_processing::prepare_and_decompile_and_recompile 31.1±1.47ms 24.9±1.18ms -19.94%
metadata_validation::validate_urls 5.2±0.01µs 4.7±0.01µs -9.62%
precise_decimal::add/0 8.6±0.00ns 8.7±0.07ns +1.16%
precise_decimal::add/rust-native 10.7±0.01ns 10.7±0.08ns 0.00%
precise_decimal::add/wasmi 275.2±0.38ns 281.4±1.01ns +2.25%
precise_decimal::add/wasmi-call-native 2.8±0.00µs 2.8±0.02µs 0.00%
precise_decimal::div/0 295.9±0.54ns 306.4±1.79ns +3.55%
precise_decimal::from_string/0 203.3±0.15ns 203.7±0.09ns +0.20%
precise_decimal::mul/0 341.4±1.45ns 336.6±0.60ns -1.41%
precise_decimal::mul/rust-native 285.1±0.55ns 286.5±0.14ns +0.49%
precise_decimal::mul/wasmi 33.8±0.27µs 34.7±0.35µs +2.66%
precise_decimal::mul/wasmi-call-native 3.2±0.01µs 3.1±0.01µs -3.13%
precise_decimal::pow/0 1788.0±8.18ns 1775.1±2.94ns -0.72%
precise_decimal::pow/rust-native 1357.2±0.59ns 1356.7±1.45ns -0.04%
precise_decimal::pow/wasmi 166.1±1.98µs 165.9±0.66µs -0.12%
precise_decimal::pow/wasmi-call-native 5.4±0.01µs 5.6±0.01µs +3.70%
precise_decimal::root/0 58.3±0.02µs 58.0±0.04µs -0.51%
precise_decimal::sub/0 9.0±0.06ns 9.0±0.10ns 0.00%
precise_decimal::to_string/0 693.0±1.07ns 697.3±0.49ns +0.62%
schema::validate_payload 382.5±0.51µs 383.5±0.28µs +0.26%
transaction::radiswap 4.9±0.03ms 4.8±0.02ms -2.04%
transaction::transfer 1838.2±4.14µs 1821.8±1.98µs -0.89%
transaction_validation::validate_manifest 43.0±0.04µs 43.1±0.05µs +0.23%
transaction_validation::verify_bls_2KB 1005.9±17.84µs 1042.7±33.41µs +3.66%
transaction_validation::verify_bls_32B 1008.0±19.53µs 1001.5±8.77µs -0.64%
transaction_validation::verify_ecdsa 74.6±0.17µs 74.5±0.05µs -0.13%
transaction_validation::verify_ed25519 42.4±0.06µs 47.1±0.12µs +11.08%

dhedey
dhedey requested changes Oct 31, 2024
View reviewed changes
Copy link
Contributor

@dhedey dhedey left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! A few things I've noticed - happy to jump on a call if you'd like to talk through any of these 👍.

radix-engine-interface/src/blueprints/resource/proof_rule.rs Show resolved Hide resolved
radix-engine-monkey-tests/src/lib.rs Show resolved Hide resolved
radix-engine-tests/tests/dec_macros/Cargo.lock Outdated Show resolved Hide resolved
radix-engine/assets/blueprints/Cargo.lock Outdated Show resolved Hide resolved
radix-transaction-scenarios/src/scenarios/account_locker.rs Show resolved Hide resolved
radix-transactions/src/builder/manifest_namer.rs Outdated Show resolved Hide resolved
radix-transactions/src/builder/manifest_namer.rs Show resolved Hide resolved
radix-transactions/src/builder/manifest_namer.rs Show resolved Hide resolved
radix-transactions/src/builder/manifest_namer.rs Show resolved Hide resolved
@dhedey dhedey force-pushed the bugfix/manifest-input-manifest-types branch from 253e552 to af29b41 Compare October 31, 2024 17:18
@0xOmarA 0xOmarA force-pushed the bugfix/manifest-input-manifest-types branch from 50c30ee to 27da7a0 Compare October 31, 2024 20:35
dhedey
dhedey approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@dhedey dhedey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! But before merging, can we fix git history (as per slack)?

radix-engine-tests/tests/system/error_injection.rs Outdated Show resolved Hide resolved
radix-engine-tests/tests/application/static_resource_movements_visitor.rs Outdated Show resolved Hide resolved
radix-engine/src/system/system_callback.rs Outdated Show resolved Hide resolved
@0xOmarA 0xOmarA force-pushed the bugfix/manifest-input-manifest-types branch from 27da7a0 to 0302118 Compare November 1, 2024 11:27
@0xOmarA 0xOmarA merged commit 9cc9eee into develop Nov 1, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhedey dhedey dhedey approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xOmarA @dhedey