Contracts: runtime_call and storage_deposit #13990

pgherveou · 2023-04-24T14:12:05Z

This PR makes the charge function fallible, so that when the the caller is unable to pay for the storage deposit we rollback the whole transaction.

frame/contracts/src/lib.rs

pgherveou · 2023-04-25T08:06:02Z

frame/contracts/src/tests.rs

+			data.encode(),
+		);
+
+		assert_err_ignore_postinfo!(result, TokenError::FundsUnavailable);


would be nice to test that the transaction gas were actually charge to the user, but I am not too sure how this can be done within our test framework

can't you just check that Alice's balance became lower than it was before the tx?

Updated the test, so we can validate that the fees are the same whether the transaction fails (because the payment failed) or not.
I don't think we can test Alice's balance because as far as I know, this would be done in the pre/post hook of the transaction_payment pallet that does not exist in the Test config

for reference

substrate/frame/transaction-payment/src/lib.rs

Lines 816 to 842 in ec43399

fn pre_dispatch(

self,

who: &Self::AccountId,

call: &Self::Call,

info: &DispatchInfoOf<Self::Call>,

len: usize,

) -> Result<Self::Pre, TransactionValidityError> {

let (_fee, imbalance) = self.withdraw_fee(who, call, info, len)?;

Ok((self.0, who.clone(), imbalance))

}

fn post_dispatch(

maybe_pre: Option<Self::Pre>,

info: &DispatchInfoOf<Self::Call>,

post_info: &PostDispatchInfoOf<Self::Call>,

len: usize,

_result: &DispatchResult,

) -> Result<(), TransactionValidityError> {

if let Some((tip, who, imbalance)) = maybe_pre {

let actual_fee = Pallet::<T>::compute_actual_fee(len as u32, info, post_info, tip);

T::OnChargeTransaction::correct_and_deposit_fee(

&who, info, post_info, actual_fee, tip, imbalance,

)?;

Pallet::<T>::deposit_event(Event::<T>::TransactionFeePaid { who, actual_fee, tip });

}

Ok(())

}

You can check the events emitted during execution. We do that in other tests. There is a event for transaction fee payment.

we would need the transaction_payment pallet for this in test, I don't think we have it 🤔 might have overlooked, will check again

Ahh yeah. You are right. We didn't set this up in tests. It is fine. We don't really need to test this.

frame/contracts/src/tests.rs

agryaznov

Just some nitpicks

frame/contracts/src/tests.rs

frame/contracts/fixtures/call_runtime_and_call.wat

agryaznov · 2023-04-25T11:10:43Z

frame/contracts/src/tests.rs

+			data.encode(),
+		);
+
+		assert_err_ignore_postinfo!(result, TokenError::FundsUnavailable);


can't you just check that Alice's balance became lower than it was before the tx?

frame/contracts/src/lib.rs

agryaznov · 2023-04-25T13:30:04Z

also some PR description would be great to have

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

frame/contracts/src/tests.rs

pgherveou · 2023-04-27T09:37:10Z

bot help

command-bot · 2023-04-27T09:37:13Z

Here's a link to docs

pgherveou · 2023-04-27T09:37:52Z

bot rebase

…runtime

paritytech-processbot · 2023-04-27T09:37:58Z

Rebased

frame/contracts/src/tests.rs

agryaznov · 2023-04-28T19:43:08Z

frame/contracts/src/storage/meter.rs

@@ -366,14 +366,14 @@ where
 	///


docs above this line could be improved a bit to reflect the fact this is now a fallible fn

pgherveou · 2023-04-29T16:07:47Z

bot merge

* wip * add comments * fix comment * comments * comments * PR comment * field orders * Update frame/contracts/src/tests.rs * Update frame/contracts/fixtures/call_runtime_and_call.wat Co-authored-by: Sasha Gryaznov <hi@agryaznov.com> * Apply suggestions from code review Co-authored-by: Sasha Gryaznov <hi@agryaznov.com> * Apply suggestions from code review Co-authored-by: Sasha Gryaznov <hi@agryaznov.com> * Update frame/contracts/src/tests.rs Co-authored-by: Sasha Gryaznov <hi@agryaznov.com> * Validate fees of failed call * Update frame/contracts/src/tests.rs * Update frame/contracts/src/tests.rs * Update frame/contracts/src/tests.rs * bubble up refund error * rename fixture file --------- Co-authored-by: Sasha Gryaznov <hi@agryaznov.com> Co-authored-by: parity-processbot <>

pgherveou requested a review from athei as a code owner April 24, 2023 14:12

wip

909cbae

pgherveou force-pushed the pg/storage_deposit_runtime branch from 62fc1e3 to 909cbae Compare April 24, 2023 14:40

add comments

fb0cc68

pgherveou added 3 commits April 24, 2023 16:59

fix comment

81859b6

comments

0ccd804

comments

a5a63da

juangirini reviewed Apr 24, 2023

View reviewed changes

frame/contracts/src/lib.rs Show resolved Hide resolved

pgherveou added 2 commits April 24, 2023 18:15

PR comment

21e2fc1

field orders

c81bde4

pgherveou commented Apr 25, 2023

View reviewed changes

frame/contracts/src/tests.rs Outdated Show resolved Hide resolved

Update frame/contracts/src/tests.rs

60c2bf4

agryaznov suggested changes Apr 25, 2023

View reviewed changes

pgherveou and others added 6 commits April 25, 2023 23:01

Update frame/contracts/fixtures/call_runtime_and_call.wat

cd177fc

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Apply suggestions from code review

672070b

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Apply suggestions from code review

ead8ba3

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Update frame/contracts/src/tests.rs

826bfa7

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Validate fees of failed call

faf1b12

Merge branch 'master' into pg/storage_deposit_runtime

ec43399

pgherveou requested a review from agryaznov April 26, 2023 08:22

pgherveou commented Apr 26, 2023

View reviewed changes

frame/contracts/src/tests.rs Outdated Show resolved Hide resolved

Update frame/contracts/src/tests.rs

fe79901

pgherveou commented Apr 26, 2023

View reviewed changes

frame/contracts/src/tests.rs Outdated Show resolved Hide resolved

Update frame/contracts/src/tests.rs

2bed075

Merge remote-tracking branch 'origin/master' into pg/storage_deposit_…

e68423d

…runtime

pgherveou commented Apr 27, 2023

View reviewed changes

frame/contracts/src/tests.rs Outdated Show resolved Hide resolved

pgherveou added 4 commits April 27, 2023 11:38

Update frame/contracts/src/tests.rs

badc93c

bubble up refund error

22cea6e

Merge branch 'master' into pg/storage_deposit_runtime

05c9f8c

rename fixture file

45aae4d

athei approved these changes Apr 28, 2023

View reviewed changes

agryaznov approved these changes Apr 28, 2023

View reviewed changes

paritytech-processbot bot merged commit bc8f2db into master Apr 29, 2023

paritytech-processbot bot deleted the pg/storage_deposit_runtime branch April 29, 2023 16:07

jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels May 2, 2023

kiltbot mentioned this pull request Jun 21, 2023

1️⃣ [AUTOMATIC] Update Polkadot dependencies from 0.9.42 to 0.9.43 KILTprotocol/kilt-node#531

Closed

kacperzuk-neti mentioned this pull request Jun 23, 2023

Polkadot v0.9.43 liberland/liberland_substrate#295

Merged

15 tasks

jasl mentioned this pull request Jun 30, 2023

Upgrade to Polkadot v0.9.43 Phala-Network/phala-blockchain#1313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contracts: runtime_call and storage_deposit #13990

Contracts: runtime_call and storage_deposit #13990

pgherveou commented Apr 24, 2023 •

edited

Loading

pgherveou Apr 25, 2023

agryaznov Apr 25, 2023

pgherveou Apr 26, 2023

athei Apr 28, 2023

pgherveou Apr 28, 2023

athei Apr 28, 2023

agryaznov left a comment

agryaznov Apr 25, 2023

agryaznov commented Apr 25, 2023

pgherveou commented Apr 27, 2023

command-bot bot commented Apr 27, 2023

pgherveou commented Apr 27, 2023

paritytech-processbot bot commented Apr 27, 2023

agryaznov Apr 28, 2023 •

edited

Loading

pgherveou commented Apr 29, 2023

	fn pre_dispatch(
	self,
	who: &Self::AccountId,
	call: &Self::Call,
	info: &DispatchInfoOf<Self::Call>,
	len: usize,
	) -> Result<Self::Pre, TransactionValidityError> {
	let (_fee, imbalance) = self.withdraw_fee(who, call, info, len)?;
	Ok((self.0, who.clone(), imbalance))
	}

	fn post_dispatch(
	maybe_pre: Option<Self::Pre>,
	info: &DispatchInfoOf<Self::Call>,
	post_info: &PostDispatchInfoOf<Self::Call>,
	len: usize,
	_result: &DispatchResult,
	) -> Result<(), TransactionValidityError> {
	if let Some((tip, who, imbalance)) = maybe_pre {
	let actual_fee = Pallet::<T>::compute_actual_fee(len as u32, info, post_info, tip);
	T::OnChargeTransaction::correct_and_deposit_fee(
	&who, info, post_info, actual_fee, tip, imbalance,
	)?;
	Pallet::<T>::deposit_event(Event::<T>::TransactionFeePaid { who, actual_fee, tip });
	}
	Ok(())
	}

Contracts: runtime_call and storage_deposit #13990

Contracts: runtime_call and storage_deposit #13990

Conversation

pgherveou commented Apr 24, 2023 • edited Loading

pgherveou Apr 25, 2023

Choose a reason for hiding this comment

agryaznov Apr 25, 2023

Choose a reason for hiding this comment

pgherveou Apr 26, 2023

Choose a reason for hiding this comment

athei Apr 28, 2023

Choose a reason for hiding this comment

pgherveou Apr 28, 2023

Choose a reason for hiding this comment

athei Apr 28, 2023

Choose a reason for hiding this comment

agryaznov left a comment

Choose a reason for hiding this comment

agryaznov Apr 25, 2023

Choose a reason for hiding this comment

agryaznov commented Apr 25, 2023

pgherveou commented Apr 27, 2023

command-bot bot commented Apr 27, 2023

pgherveou commented Apr 27, 2023

paritytech-processbot bot commented Apr 27, 2023

agryaznov Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

pgherveou commented Apr 29, 2023

pgherveou commented Apr 24, 2023 •

edited

Loading

agryaznov Apr 28, 2023 •

edited

Loading