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

Fix proof name reservation #1635

Merged
merged 3 commits into from
Oct 27, 2023
Merged

Fix proof name reservation #1635

merged 3 commits into from
Oct 27, 2023

Conversation

lrubasze
Copy link
Contributor

Use proper method when reserving the proof name.

@github-actions
Copy link

github-actions bot commented Oct 26, 2023

Benchmark for 51c2306

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 76.8±0.33ms 75.6±0.32ms -1.56%
costing::decode_sbor 16.7±0.04µs 17.1±0.11µs +2.40%
costing::decode_sbor_bytes 51.3±0.07µs 47.2±0.09µs -7.99%
costing::deserialize_wasm 1372.9±44.81µs 1386.9±3.87µs +1.02%
costing::instantiate_flash_loan 4.2±0.56ms 4.6±0.91ms +9.52%
costing::instantiate_radiswap 6.7±0.06ms 6.7±0.07ms 0.00%
costing::spin_loop 26.0±0.10ms 27.3±0.08ms +5.00%
costing::validate_sbor_payload 32.2±0.05µs 30.6±0.11µs -4.97%
costing::validate_sbor_payload_bytes 381.2±0.64ns 392.2±2.01ns +2.89%
costing::validate_secp256k1 87.6±0.33µs 87.6±0.32µs 0.00%
costing::validate_wasm 43.7±0.10ms 43.2±0.11ms -1.14%
decimal::add/0 9.3±0.00ns 9.3±0.00ns 0.00%
decimal::add/rust-native 10.8±0.01ns 10.8±0.00ns 0.00%
decimal::add/wasmer 133.1±0.79ns 127.2±0.14ns -4.43%
decimal::add/wasmer-call-native 554.9±1.01ns 578.0±2.22ns +4.16%
decimal::add/wasmi 826.9±3.40ns 842.2±3.15ns +1.85%
decimal::add/wasmi-call-native 8.1±0.00µs 8.2±0.01µs +1.23%
decimal::div/0 202.1±0.36ns 199.8±0.50ns -1.14%
decimal::from_string/0 174.4±0.44ns 173.8±0.57ns -0.34%
decimal::mul/0 155.7±0.24ns 158.2±0.08ns +1.61%
decimal::mul/rust-native 154.5±0.23ns 154.0±0.12ns -0.32%
decimal::mul/wasmer 1700.0±1.03ns 1688.6±2.14ns -0.67%
decimal::mul/wasmer-call-native 721.0±0.35ns 722.2±1.93ns +0.17%
decimal::mul/wasmi 60.9±0.41µs 60.8±0.04µs -0.16%
decimal::mul/wasmi-call-native 8.2±0.01µs 8.3±0.01µs +1.22%
decimal::pow/0 724.1±0.40ns 732.6±1.40ns +1.17%
decimal::pow/rust-native 701.1±0.78ns 701.0±1.36ns -0.01%
decimal::pow/wasmer 7.3±0.00µs 7.3±0.00µs 0.00%
decimal::pow/wasmer-call-native 1199.8±1.85ns 1194.9±1.31ns -0.41%
decimal::pow/wasmi 286.5±0.66µs 285.7±0.71µs -0.28%
decimal::pow/wasmi-call-native 7.4±0.01µs 7.4±0.00µs 0.00%
decimal::root/0 9.8±0.01µs 8.9±0.02µs -9.18%
decimal::sub/0 9.4±0.01ns 9.4±0.01ns 0.00%
decimal::to_string/0 478.8±0.43ns 480.1±0.96ns +0.27%
precise_decimal::add/0 11.1±0.01ns 10.4±0.08ns -6.31%
precise_decimal::add/rust-native 12.7±0.00ns 12.7±0.00ns 0.00%
precise_decimal::add/wasmer 136.9±0.20ns 135.5±0.07ns -1.02%
precise_decimal::add/wasmer-call-native 588.5±0.66ns 578.8±0.47ns -1.65%
precise_decimal::add/wasmi 1018.6±2.20ns 1057.9±5.09ns +3.86%
precise_decimal::add/wasmi-call-native 8.6±0.01µs 8.6±0.01µs 0.00%
precise_decimal::div/0 330.0±2.68ns 325.7±0.72ns -1.30%
precise_decimal::from_string/0 222.1±0.30ns 224.4±0.64ns +1.04%
precise_decimal::mul/0 346.4±0.59ns 348.4±0.71ns +0.58%
precise_decimal::mul/rust-native 328.9±2.14ns 327.0±0.78ns -0.58%
precise_decimal::mul/wasmer 3.9±0.00µs 3.8±0.00µs -2.56%
precise_decimal::mul/wasmer-call-native 961.5±0.91ns 957.8±0.77ns -0.38%
precise_decimal::mul/wasmi 152.8±0.23µs 152.2±0.08µs -0.39%
precise_decimal::mul/wasmi-call-native 10.3±0.03µs 9.2±0.01µs -10.68%
precise_decimal::pow/0 1927.8±6.08ns 1926.1±2.09ns -0.09%
precise_decimal::pow/rust-native 1586.0±3.74ns 1579.1±5.54ns -0.44%
precise_decimal::pow/wasmer 18.1±0.01µs 18.1±0.02µs 0.00%
precise_decimal::pow/wasmer-call-native 2.3±0.00µs 2.3±0.00µs 0.00%
precise_decimal::pow/wasmi 735.5±1.44µs 740.8±1.00µs +0.72%
precise_decimal::pow/wasmi-call-native 16.8±0.03µs 16.8±0.03µs 0.00%
precise_decimal::root/0 66.1±0.03µs 64.7±0.02µs -2.12%
precise_decimal::sub/0 10.5±0.01ns 10.5±0.01ns 0.00%
precise_decimal::to_string/0 771.6±2.62ns 776.8±1.27ns +0.67%
schema::validate_payload 350.5±19.97µs 347.9±0.47µs -0.74%
transaction::radiswap 6.7±0.07ms 6.5±0.07ms -2.99%
transaction::transfer 2.0±0.00ms 2.0±0.01ms 0.00%
transaction_processing::prepare 3.0±0.00ms 2.8±0.00ms -6.67%
transaction_processing::prepare_and_decompile 7.1±0.03ms 7.2±0.02ms +1.41%
transaction_processing::prepare_and_decompile_and_recompile 26.4±0.15ms 27.2±0.10ms +3.03%
transaction_validation::validate_manifest 48.7±0.05µs 48.9±0.60µs +0.41%
transaction_validation::verify_ecdsa 85.1±0.09µs 85.2±0.28µs +0.12%
transaction_validation::verify_ed25519 61.3±4.47µs 61.1±0.29µs -0.33%

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.

Nice, thanks for the fix 👍

.then(|builder| {
let lookup = builder.name_lookup();
let proof_id = lookup.proof("proof");
builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this test name, I'd expect this to use the clone proof command, rather than advanced instructions.

Perhaps it would be a more explicit to create a test file for the manifest builder itself, and explicitly add tests for the "add_instruction_advanced" command, which checks it is collision resistant against existing proofs/buckets named "proof" and "bucket"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that sounds good. Will do that.

@lrubasze lrubasze force-pushed the fix/proof_name_reservation branch from 0ee54bc to d179cdf Compare October 26, 2023 16:11
@lrubasze lrubasze merged commit 015f520 into develop Oct 27, 2023
25 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.

3 participants