Enable Merklized Distributions in Assets Pallet #5400

shawntabrizi · 2024-08-19T02:27:50Z

Depends on: #3881

This PR introduces a way for an asset issuer to distribute their token to many users using a single merkle root.

Users then present a merkle proof which is verified, and triggers the do_mint function.

A tracking storage ensures that each user only claims their distribution a single time.

The design allows for multiple distributions per asset id.

This reverts commit 1cfb29f.

Co-authored-by: Ankan <[email protected]>

…abrizi/polkadot-sdk into shawntabrizi-proving-trie

shawntabrizi · 2024-08-19T02:31:41Z

This PR is blocked because the data structures in binary-merkle-tree crate are not usable in the runtime because usize does not work with Encode, Decode, or TypeInfo.

kianenigma · 2024-09-19T13:21:57Z

cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_assets_pool.rs

@@ -538,4 +538,20 @@ impl<T: frame_system::Config> pallet_assets::WeightInfo for WeightInfo<T> {
 			.saturating_add(T::DbWeight::get().reads(1_u64))
 			.saturating_add(T::DbWeight::get().writes(1_u64))
 	}
+
+	fn mint_distribution() -> Weight {
+		Weight::default()


TODO prior to merge

substrate/frame/assets/src/benchmarking.rs

substrate/frame/assets/src/lib.rs

kianenigma · 2024-09-19T13:25:49Z

substrate/frame/assets/src/types.rs

+
+#[derive(Eq, PartialEq, Copy, Clone, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)]
+pub struct DistributionInfo<AssetId, Hash> {
+	// The asset id we are distributing.
+	pub asset_id: AssetId,
+	// The merkle root which represents all the balances to distribute.
+	pub merkle_root: Hash,
+	// Whether the distribution is still active.
+	pub active: bool,
+}


Suggested change

#[derive(Eq, PartialEq, Copy, Clone, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)]

pub struct DistributionInfo<AssetId, Hash> {

// The asset id we are distributing.

pub asset_id: AssetId,

// The merkle root which represents all the balances to distribute.

pub merkle_root: Hash,

// Whether the distribution is still active.

pub active: bool,

}

/// Foo bar baz

#[derive(Eq, PartialEq, Copy, Clone, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)]

pub struct DistributionInfo<AssetId, Hash> {

/// The asset id we are distributing.

pub asset_id: AssetId,

/// The merkle root which represents all the balances to distribute.

pub merkle_root: Hash,

/// Whether the distribution is still active.

pub active: bool,

}

kianenigma · 2024-09-19T13:26:36Z

substrate/frame/assets/src/lib.rs

@@ -1798,6 +1848,92 @@ pub mod pallet {
 			)?;
 			Ok(())
 		}
+
+		/// Mint a distribution of assets of a particular class.


Something about this can go into the main top docs and then in https://docs.rs/pallet-assets/latest/pallet_assets/?

kianenigma

Some small nits remaining, but the overall logic is good

Ank4n

First pass. Haven't looked yet at the proving trie code.

substrate/frame/assets/src/lib.rs

substrate/frame/assets/src/functions.rs

Ank4n · 2024-09-19T21:06:50Z

substrate/frame/assets/src/functions.rs

+		if all_refunded {
+			Self::deposit_event(Event::<T, I>::DistributionCleaned { distribution_id });
+			// Refund weight only the amount we actually used.
+			Ok(Some(T::WeightInfo::destroy_distribution(refund_count)).into())


What is the incentive to clean up state? Shouldn't we refund all fees?

anyone can clean up state... but yeah we could add a deposit here for doing a distribution

substrate/frame/assets/src/functions.rs

Ank4n · 2024-09-19T21:10:28Z

substrate/frame/assets/src/lib.rs

+		) -> DispatchResult {
+			let origin = ensure_signed(origin)?;
+			let id: T::AssetId = id.into();
+			Self::do_mint_distribution(id, merkle_root, Some(origin))?;


Thinking out loud: Should we not take some kind of deposit here (charging for the storage), that is only refunded when state (MerklizedDistributionTracker) is cleaned up? What stops some one to create large number of distributions?

substrate/frame/assets/src/lib.rs

…/shawntabrizi/polkadot-sdk into pr/5400

Co-authored-by: Ankan <[email protected]>

substrate/frame/assets/src/lib.rs

Ank4n · 2024-10-28T19:39:03Z

substrate/frame/assets/src/functions.rs

+			ensure!(check_issuer == details.issuer, Error::<T, I>::NoPermission);
+		}
+
+		let info = DistributionInfo {


Perhaps you have already considered this, but I'm still a little concerned that an account can create a bunch of distributions without paying any storage cost.

We could limit this by either

Requiring a deposit when creating a distribution.

Or, limit it to one active distribution per asset ID.

wdyt?

If we don't expect multiple active distribution for an asset, probably that is the most straightforward. We can use AssetId as the key for MerklizedDistribution.

i am okay with you adding storage deposit to this PR. i would like to hand it off to someone else to own and take to finish line

Ank4n · 2024-10-28T19:47:10Z

substrate/frame/assets/src/lib.rs

+
+	#[pallet::storage]
+	/// Merklized distribution of an asset.
+	pub(super) type MerklizedDistribution<T: Config<I>, I: 'static = ()> = StorageMap<


nit: could be a CountedStorageMap to count active distributions.

no, because the count would go down when we clean up a distribution. and we want the count to remain unique

Ank4n · 2024-10-28T19:51:32Z

substrate/frame/assets/src/lib.rs

+		///
+		/// - `distribution_id`: The identifier of the distribution.
+		/// - `merkle_proof`: The merkle proof of the account and balance in a compact base-16
+		///   merkle trie used to authorize minting.


missing info about hashes?

Ank4n

Just one concern about preventing distribution spam, otherwise happy to approve.

shawntabrizi and others added 28 commits March 28, 2024 14:05

initial ideas

d50d5e4

Update proving_trie.rs

7f4422e

create trait

bdc0c84

use trait

da385ab

clean up trait and basic trie further

6ab4bfb

use trait in session historical

1cfb29f

fix api, add basic end to end test

3080c7b

fix test

39b0fca

Revert "use trait in session historical"

599f576

This reverts commit 1cfb29f.

Merge branch 'master' into shawntabrizi-proving-trie

a1c8886

Merge branch 'master' into shawntabrizi-proving-trie

6bde2a3

Update substrate/primitives/runtime/src/proving_trie.rs

ead7951

Co-authored-by: Ankan <[email protected]>

Merge branch 'master' into shawntabrizi-proving-trie

0cac048

fix some feedback

acfe734

update name

45f4287

docs and multi value proof

efbe1c9

improve test

cfa62ce

add multi-value query test

885fd94

Merge branch 'master' into shawntabrizi-proving-trie

18a5c2e

Create pr_3881.prdoc

5e3e518

Merge branch 'shawntabrizi-proving-trie' of https://github.com/shawnt…

f35cacc

…abrizi/polkadot-sdk into shawntabrizi-proving-trie

use v1

e6c3759

Merge branch 'master' into shawntabrizi-proving-trie

89679c3

initial idea

b40c96a

some bs code to make things compile

37052ee

more stuff

08f2b35

complete logic

118a51c

add verification check

44bc5bb

Merge branch 'master' into shawntabrizi-proving-trie

6e853ba

Merge branch 'master' into shawntabrizi-assets-distribution

42259eb

kianenigma reviewed Sep 19, 2024

View reviewed changes

substrate/frame/assets/src/benchmarking.rs Outdated Show resolved Hide resolved

kianenigma reviewed Sep 19, 2024

View reviewed changes

substrate/frame/assets/src/lib.rs Outdated Show resolved Hide resolved

kianenigma reviewed Sep 19, 2024

View reviewed changes

kianenigma approved these changes Sep 19, 2024

View reviewed changes

Ank4n reviewed Sep 19, 2024

View reviewed changes

Merge branch 'master' into pr/5400

2f1f18a

paritytech-review-bot bot requested a review from a team September 23, 2024 21:47

shawntabrizi and others added 13 commits September 30, 2024 07:25

Merge branch 'master' into pr/5400

72dd0a1

fix build

860f346

Merge branch 'master' into pr/5400

1c306f6

Merge branch 'shawntabrizi-assets-distribution' of https://github.com…

3693527

…/shawntabrizi/polkadot-sdk into pr/5400

weird rust trait stuff

528e71f

Merge branch 'master' into pr/5400

b1e5387

fix no default

9f108c3

allow default as no trie

fe2e08a

use separate count

2adc76f

remove map at the end of delete

22f26d9

Update substrate/frame/assets/src/lib.rs

c0a4032

Co-authored-by: Ankan <[email protected]>

remove dead code

15be9c8

Merge branch 'master' into pr/5400

8b5701c

kianenigma requested review from Ank4n and muharem October 25, 2024 10:38

kianenigma reviewed Oct 25, 2024

View reviewed changes

substrate/frame/assets/src/lib.rs Show resolved Hide resolved

Ank4n reviewed Oct 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Merklized Distributions in Assets Pallet #5400

Enable Merklized Distributions in Assets Pallet #5400

shawntabrizi commented Aug 19, 2024 •

edited

Loading

shawntabrizi commented Aug 19, 2024

kianenigma Sep 19, 2024

kianenigma Sep 19, 2024

kianenigma Sep 19, 2024

kianenigma left a comment

Ank4n left a comment

Ank4n Sep 19, 2024

shawntabrizi Oct 2, 2024

Ank4n Sep 19, 2024

Ank4n Oct 28, 2024

Ank4n Oct 28, 2024

shawntabrizi Oct 28, 2024

Ank4n Oct 28, 2024

shawntabrizi Oct 28, 2024

Ank4n Oct 28, 2024

Ank4n left a comment

Enable Merklized Distributions in Assets Pallet #5400

Are you sure you want to change the base?

Enable Merklized Distributions in Assets Pallet #5400

Conversation

shawntabrizi commented Aug 19, 2024 • edited Loading

shawntabrizi commented Aug 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

Ank4n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ank4n left a comment

Choose a reason for hiding this comment

shawntabrizi commented Aug 19, 2024 •

edited

Loading