Make api and primitives generic over Runtime #340

haerdib · 2022-11-24T11:25:09Z

Types defined by the Runtime (Hash, Index, Balance .. ) are now not hardcoded any more, but depend on the Runtime input.
Renamed generic parameter P (probably for Pair) to Signer
Removed pallet dependencies completely from no_std due to default wasm compilation.

echevrier

LGTM

add traits add balanes impl add Runtime trait make api generic over Runtime remove interfaces fix clippy fix staking feature fix doc make RuntimeVersion generic over Runtime revert last commit fix examples

haerdib · 2022-11-30T08:07:55Z

node-api/src/events.rs

@@ -180,7 +180,7 @@ impl EventDetails {

 		// topics come after the event data in EventRecord. They aren't used for
 		// anything at the moment, so just decode and throw them away.
-		let _topics = Vec::<Hash>::decode(input)?;
+		let _topics = Vec::<H256>::decode(input)?;


Hardcoded H256 for now.. as they're not used and are unlikely to be different from H256 at the moment.

haerdib · 2022-11-30T08:08:57Z

primitives/src/lib.rs

@@ -22,20 +22,3 @@ pub use extrinsics::*;

 pub mod extrinsic_params;
 pub mod extrinsics;
-
-/// The block number type used in this runtime.
-pub type BlockNumber = u64;


Not needed anymore, because all these params should now be generic inside the api-client.

haerdib · 2022-11-30T08:15:50Z

src/api/api_client.rs

 		self.signer = Some(signer);
 	}

 	/// Get the public part of the api signer account.
-	pub fn signer_account(&self) -> Option<AccountId> {
+	pub fn signer_account(&self) -> Option<AccountId32> {
 		let pair = self.signer.as_ref()?;
 		let multi_signer = MultiSigner::from(pair.public());
 		Some(multi_signer.into_account())


MultiSigner only implements into_account for AccountId32. Not sure why, but I guess there's a reason.

Yeah, because moonbeam does in fact implement an account id with 20 bytes, but still use the multisigner if I am not mistaken.

clangenb

Nice to see everything getting so generic. However, I am unsure if depending on the runtime directly is a potential blocker for some use cases, especially the no-std world because it compiles to wasm in no-std. So maybe we need to have a layer in between like:

pub trait RuntimeConfig {
    type Balance;

    // etc.

   // We could even define the tips here
}

// We could add feature gated configs like:
impl RuntimeConfig for KitchenSinkRuntime {
   type Balance = KitchenSinkRuntime::Balance

   // etc
}

I am not sure if this creates problems on other ends though.

examples/staking_payout.rs

clangenb · 2022-12-05T13:40:58Z

The pallets-itself also assume wasm in no-std, so in my opinion, you can't get around the runtime config trait that I am suggesting. So if you want to merge this PR now, I suggest fixing the trait immediately afterwards, so we don't have a breaking interface in subsequent releases.

…/scs/substrate-api-client into bh/make-api-generic-over-runtime

clangenb

Nice looks good now. I like the approach! :)

Only a few minor remarks are left.

clangenb · 2022-12-06T14:19:14Z

primitives/src/types.rs

+//! Re-defintion of substrate primitives.
+//! Needed because substrate pallets compile to wasm in no_std.
+


Maybe we can note that all these primitives are from the pallet-balances, aren't they?

They are not. Should I paste a link from where they are taken?

Yeah, this would be fabulous! 👍

clangenb · 2022-12-06T14:21:25Z

src/api/api_client.rs

 		self.signer = Some(signer);
 	}

 	/// Get the public part of the api signer account.
-	pub fn signer_account(&self) -> Option<AccountId> {
+	pub fn signer_account(&self) -> Option<AccountId32> {
 		let pair = self.signer.as_ref()?;
 		let multi_signer = MultiSigner::from(pair.public());
 		Some(multi_signer.into_account())


Yeah, because moonbeam does in fact implement an account id with 20 bytes, but still use the multisigner if I am not mistaken.

src/api/api_client.rs

primitives/src/pallet_traits/pallet_staking_config.rs

clangenb

Nice, looks good to me!

haerdib self-assigned this Nov 28, 2022

haerdib mentioned this pull request Nov 28, 2022

Investigate: can the tip type be detected via metadata or Runtime? #344

Closed

haerdib added F7-enhancement Enhances an already existing functionality E2-breaksapi labels Nov 29, 2022

haerdib changed the title ~~Make api generic over runtime~~ Make api generic over Runtime Nov 29, 2022

haerdib marked this pull request as ready for review November 29, 2022 07:34

haerdib requested review from echevrier and clangenb November 29, 2022 07:34

echevrier approved these changes Nov 29, 2022

View reviewed changes

haerdib force-pushed the bh/make-api-generic-over-runtime branch 2 times, most recently from 7dae69d to ba25bfc Compare November 29, 2022 13:10

add interface traits

4679f17

add traits add balanes impl add Runtime trait make api generic over Runtime remove interfaces fix clippy fix staking feature fix doc make RuntimeVersion generic over Runtime revert last commit fix examples

haerdib force-pushed the bh/make-api-generic-over-runtime branch from ba25bfc to 4679f17 Compare November 29, 2022 13:20

haerdib marked this pull request as draft November 29, 2022 14:13

haerdib added 4 commits November 29, 2022 15:19

fix rebase changes

91c76d0

add missing contracts

af921dd

amke staking generic over BalanceType

a44d07a

cargo fmt

b173d7c

haerdib marked this pull request as ready for review November 29, 2022 15:00

haerdib marked this pull request as draft November 29, 2022 15:03

haerdib changed the title ~~Make api generic over Runtime~~ Make api and primitives generic over Runtime Nov 29, 2022

haerdib added 3 commits November 29, 2022 17:11

make these types truly generic

b433e91

cargo fmt

6ab7914

fix tests

2cb2d7d

haerdib marked this pull request as ready for review November 30, 2022 08:05

haerdib commented Nov 30, 2022

View reviewed changes

Merge branch 'master' into bh/make-api-generic-over-runtime

e8fa803

haerdib requested review from clangenb and removed request for clangenb December 1, 2022 07:19

clangenb reviewed Dec 1, 2022

View reviewed changes

examples/staking_payout.rs Outdated Show resolved Hide resolved

Merge branch 'master' into bh/make-api-generic-over-runtime

b2bf4aa

haerdib requested review from clangenb and removed request for clangenb December 5, 2022 09:28

add pallets trait

72fffe2

haerdib marked this pull request as draft December 5, 2022 16:47

haerdib added 16 commits December 5, 2022 18:38

add no_std traits

6abdeed

remove obsoltete sp_std

cea2b99

Merge branch 'bh/make-api-generic-over-runtime' of https://github.com…

8657599

…/scs/substrate-api-client into bh/make-api-generic-over-runtime

add missing re-export

043d5fb

remove frame-support and frame-system from node-api

1bb52e7

add pallet contracts

bfa7d1e

add pallets contracts

974034b

add std impl for pallet traits

8829d45

add pallet staking

8fcb8ce

remove pallet dependencies

c1b7368

add tpyes.rs

d34f317

cargo fmt

f467b05

fix taplo

cfc7fe2

remove substrate comment

634f9af

fix toml formatting

10c2d5c

fix taplo

4b26c5c

haerdib marked this pull request as ready for review December 6, 2022 13:00

haerdib requested review from clangenb and removed request for clangenb December 6, 2022 13:00

clangenb reviewed Dec 6, 2022

View reviewed changes

add links to the types where taken from substrate

7df61e7

clangenb approved these changes Dec 6, 2022

View reviewed changes

haerdib merged commit d540964 into master Dec 6, 2022

haerdib deleted the bh/make-api-generic-over-runtime branch December 6, 2022 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make api and primitives generic over Runtime #340

Make api and primitives generic over Runtime #340

haerdib commented Nov 24, 2022 •

edited

Loading

echevrier left a comment

haerdib Nov 30, 2022

haerdib Nov 30, 2022

haerdib Nov 30, 2022

clangenb Dec 6, 2022 •

edited

Loading

clangenb left a comment •

edited

Loading

clangenb commented Dec 5, 2022 •

edited

Loading

clangenb left a comment

clangenb Dec 6, 2022

haerdib Dec 6, 2022

clangenb Dec 6, 2022

haerdib Dec 6, 2022

clangenb Dec 6, 2022 •

edited

Loading

clangenb left a comment

		//! Re-defintion of substrate primitives.
		//! Needed because substrate pallets compile to wasm in no_std.

Make api and primitives generic over Runtime #340

Make api and primitives generic over Runtime #340

Conversation

haerdib commented Nov 24, 2022 • edited Loading

echevrier left a comment

Choose a reason for hiding this comment

haerdib Nov 30, 2022

Choose a reason for hiding this comment

haerdib Nov 30, 2022

Choose a reason for hiding this comment

haerdib Nov 30, 2022

Choose a reason for hiding this comment

clangenb Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

clangenb left a comment • edited Loading

Choose a reason for hiding this comment

clangenb commented Dec 5, 2022 • edited Loading

clangenb left a comment

Choose a reason for hiding this comment

clangenb Dec 6, 2022

Choose a reason for hiding this comment

haerdib Dec 6, 2022

Choose a reason for hiding this comment

clangenb Dec 6, 2022

Choose a reason for hiding this comment

haerdib Dec 6, 2022

Choose a reason for hiding this comment

clangenb Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

clangenb left a comment

Choose a reason for hiding this comment

haerdib commented Nov 24, 2022 •

edited

Loading

clangenb Dec 6, 2022 •

edited

Loading

clangenb left a comment •

edited

Loading

clangenb commented Dec 5, 2022 •

edited

Loading

clangenb Dec 6, 2022 •

edited

Loading