Skip to content

Contracts: Fix terminate benchmark #3558

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

Merged
pgherveou merged 8 commits into from
Mar 8, 2024
Merged

Contracts: Fix terminate benchmark #3558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
37b5419
Contracts: Fix delegate_dependency length in benchmark
pgherveou Mar 4, 2024
7b2721a
Merge branch 'master' into pg/contracts-fix-delegate_dependency-lengt…
pgherveou Mar 4, 2024
a459758
Apply suggestions from code review
pgherveou Mar 5, 2024
613d228
Add assert
pgherveou Mar 5, 2024
9dd7b67
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
Mar 5, 2024
4258c5a
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
Mar 5, 2024
c6a8491
Merge branch 'master' into pg/contracts-fix-delegate_dependency-lengt…
athei Mar 6, 2024
e615d67
Merge branch 'master' into pg/contracts-fix-delegate_dependency-lengt…
pgherveou Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion substrate/frame/contracts/src/benchmarking/mod.rs
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@pgherveou pgherveou Mar 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We could also introduce a seal_terminate_per_locked_dependency but the extra PoV should be pretty low and will be refunded anyway with PoV Reclaim

Copy link
Copy Markdown
Member

@athei athei Mar 5, 2024

Choose a reason for hiding this comment

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

Let's wait for the benchmarks to see how bad it is. I don't think the reclaim is working, yet. The linked PR is only the node side. However, terminate is rare enough so that I wouldn't optimize too much for it so we are probably good.

Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ benchmarks! {
value: code_hashes_bytes,
},
],
deploy_body: Some(body::repeated_dyn(r, vec![
deploy_body: Some(body::repeated_dyn(T::MaxDelegateDependencies::get(), vec![
Counter(beneficiary_len as u32, code_hash_len as u32), // code_hash_ptr
Regular(Instruction::Call(1)),
])),
Expand All @@ -943,6 +943,7 @@ benchmarks! {
assert_eq!(T::Currency::total_balance(&beneficiary), 0u32.into());
assert_eq!(T::Currency::balance(&instance.account_id), Pallet::<T>::min_balance() * 2u32.into());
assert_ne!(T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &instance.account_id), 0u32.into());
assert_eq!(ContractInfoOf::<T>::get(&instance.account_id).unwrap().delegate_dependencies_count() as u32, T::MaxDelegateDependencies::get());
}: call(origin, instance.addr.clone(), 0u32.into(), Weight::MAX, None, vec![])
verify {
if r > 0 {
Expand Down
5 changes: 5 additions & 0 deletions substrate/frame/contracts/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ impl<T: Config> ContractInfo<T> {
Ok(contract)
}

/// Returns the number of locked delegate dependencies.
pub fn delegate_dependencies_count(&self) -> usize {
self.delegate_dependencies.len()
}

/// Associated child trie unique id is built from the hash part of the trie id.
pub fn child_trie_info(&self) -> ChildInfo {
ChildInfo::new_default(self.trie_id.as_ref())
Expand Down