fix: only allow next authority to create billing extrinsics (#558)

Author: DylanVerstraete

substrate-node/Cargo.lock

@@ -2,5 +2,5 @@ apiVersion: v2
 name: substrate-node
 description: Tfchain node
 type: application
-version: 0.2.6
+version: 0.2.7
 appVersion: '2.2.0-rc5'

substrate-node/charts/substrate-node/Chart.yaml

@@ -155,25 +155,6 @@ spec:
             - name: keys
               mountPath: /keys
               readOnly: true
-        - name: insert-smct-key
-          securityContext:
-            {{- toYaml .Values.securityContext | nindent 12 }}
-          image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-          imagePullPolicy: {{ .Values.image.pullPolicy }}
-          args: [
-            "key",
-            "insert",
-            "--keystore-path=/keystore",
-            "--key-type", "smct",
-            "--suri","/keys/smct",
-            "--scheme=sr25519"
-          ]
-          volumeMounts:
-            - name: keystore
-              mountPath: /keystore
-            - name: keys
-              mountPath: /keys
-              readOnly: true
       {{ end }}
       volumes:
         - name: keystore

substrate-node/charts/substrate-node/templates/deployment.yaml

@@ -5,11 +5,11 @@
 image:
   repository: tfchainnode
   pullPolicy: IfNotPresent
-  tag: ""
+  tag: ''
 
 imagePullSecrets: []
-nameOverride: ""
-fullnameOverride: ""
+nameOverride: ''
+fullnameOverride: ''
 
 serviceAccount:
   # Specifies whether a service account should be created
@@ -18,7 +18,7 @@ serviceAccount:
   annotations: {}
   # The name of the service account to use.
   # If not set and create is true, a name is generated using the fullname template
-  name: ""
+  name: ''
 
 podAnnotations: {}
 
@@ -39,15 +39,15 @@ service:
   type: ClusterIP
   port: 80
 
-name: "tfchainnode01"
+name: 'tfchainnode01'
 port: 30333
 ws_port: 9944
 rpc_port: 9933
 is_validator: true
-chainspec: "/etc/chainspecs/dev/chainSpecRaw.json"
+chainspec: '/etc/chainspecs/dev/chainSpecRaw.json'
 ws_max_connections: 1048576
 #rpc_methods: "Unsafe"
-rpc_methods: "safe"
+rpc_methods: 'safe'
 disable_offchain_worker: true
 # telemetry_url: ""
 
@@ -60,9 +60,6 @@ keys: []
 #   secret: 1a...
 # - name: tft
 #   secret: "kkjghjfkkj kjhgkkhhgg"
-## Smart contract billing offchain worker key
-# - name: smct
-#   secret: "fsfdsfsdf"
 
 # boot_node: "/ip4/10.42.1.134/tcp/30333/p2p/12D3KooWGX8JFxZu2dDmGVpa9t9enZnFCLhH4NUBA7PDuhEVQTMg"
 
@@ -94,10 +91,10 @@ resources:
 
 volume:
   size: 10Gi
-  existingpersistentVolumeClaim: ""
+  existingpersistentVolumeClaim: ''
   persistentVolume:
     create: true
-    hostPath: "/chain01"
+    hostPath: '/chain01'
 
 nodeSelector: {}
 
@@ -106,4 +103,4 @@ tolerations: []
 affinity: {}
 
 threefoldVdc:
-  backup: ""
+  backup: ''

substrate-node/charts/substrate-node/values.yaml

@@ -42,6 +42,8 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0
 frame-try-runtime = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.24", default-features = false, optional = true }
 sp-core = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.24", default-features = false }
 pallet-authorship = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.24", default-features = false }
+pallet-session = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.24", default-features = false }
+
 parking_lot = '0.12.1'
 
 # Benchmarking
@@ -50,6 +52,7 @@ frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch =
 [dev-dependencies]
 sp-keystore =  { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.24", default-features = false }
 env_logger = "*"
+substrate-validator-set = { path = "../substrate-validator-set" }
 
 [features]
 default = ['std']

substrate-node/pallets/pallet-smart-contract/Cargo.toml

@@ -19,7 +19,6 @@ use frame_system::{
     self as system, ensure_signed,
     offchain::{AppCrypto, CreateSignedTransaction, SendSignedTransaction, Signer},
 };
-
 pub use pallet::*;
 use pallet_authorship;
 use pallet_tfgrid;
@@ -28,7 +27,7 @@ use pallet_tfgrid::types as pallet_tfgrid_types;
 use pallet_timestamp as timestamp;
 use sp_core::crypto::KeyTypeId;
 use sp_runtime::{
-    traits::{CheckedSub, SaturatedConversion},
+    traits::{CheckedSub, Convert, SaturatedConversion},
     Perbill,
 };
 use substrate_fixed::types::U64F64;
@@ -38,7 +37,7 @@ use tfchain_support::{
     types::PublicIP,
 };
 
-pub const KEY_TYPE: KeyTypeId = KeyTypeId(*b"smct");
+pub const KEY_TYPE: KeyTypeId = KeyTypeId(*b"aura");
 
 #[cfg(test)]
 mod mock;
@@ -211,6 +210,7 @@ pub mod pallet {
         + pallet_tfgrid::Config
         + pallet_tft_price::Config
         + pallet_authorship::Config
+        + pallet_session::Config
     {
         type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
         type Currency: LockableCurrency<Self::AccountId>;
@@ -341,7 +341,6 @@ pub mod pallet {
         NodeNotAvailableToDeploy,
         CannotUpdateContractInGraceState,
         NumOverflow,
-        OffchainSignedTxNotBlockAuthor,
         OffchainSignedTxCannotSign,
         OffchainSignedTxAlreadySent,
         OffchainSignedTxNoLocalAccountAvailable,
@@ -351,6 +350,8 @@ pub mod pallet {
         NoSuchSolutionProvider,
         SolutionProviderNotApproved,
         CanOnlyIncreaseFrequency,
+        IsNotAnAuthority,
+        WrongAuthority,
     }
 
     #[pallet::genesis_config]
@@ -941,8 +942,8 @@ impl<T: Config> Pallet<T> {
     fn bill_contract_using_signed_transaction(contract_id: u64) -> Result<(), Error<T>> {
         let signer = Signer::<T, <T as pallet::Config>::AuthorityId>::any_account();
 
-        // Only allow the author of the block to trigger the billing
-        Self::is_block_author(&signer)?;
+        // Only allow the author of the next block to trigger the billing
+        Self::is_next_block_author(&signer)?;
 
         if !signer.can_sign() {
             log::error!(
@@ -1795,14 +1796,36 @@ impl<T: Config> PublicIpModifier for Pallet<T> {
 }
 
 impl<T: Config> Pallet<T> {
-    fn is_block_author(signer: &Signer<T, <T as Config>::AuthorityId>) -> Result<(), Error<T>> {
+    // Validates if the given signer is the next block author based on the validators in session
+    // This can be used if an extrinsic should be refunded by the author in the same block
+    // It also requires that the keytype inserted for the offchain workers is the validator key
+    fn is_next_block_author(
+        signer: &Signer<T, <T as Config>::AuthorityId>,
+    ) -> Result<(), Error<T>> {
         let author = <pallet_authorship::Pallet<T>>::author();
+        let validators = <pallet_session::Pallet<T>>::validators();
 
+        // Sign some arbitrary data in order to get the AccountId, maybe there is another way to do this?
         let signed_message = signer.sign_message(&[0]);
         if let Some(signed_message_data) = signed_message {
             if let Some(block_author) = author {
-                if signed_message_data.0.id != block_author {
-                    return Err(Error::<T>::OffchainSignedTxNotBlockAuthor);
+                let validator =
+                    <T as pallet_session::Config>::ValidatorIdOf::convert(block_author.clone())
+                        .ok_or(Error::<T>::IsNotAnAuthority)?;
+
+                let validator_count = validators.len();
+                let author_index = (validators.iter().position(|a| a == &validator).unwrap_or(0)
+                    + 1)
+                    % validator_count;
+
+                let signer_validator_account =
+                    <T as pallet_session::Config>::ValidatorIdOf::convert(
+                        signed_message_data.0.id.clone(),
+                    )
+                    .ok_or(Error::<T>::IsNotAnAuthority)?;
+
+                if signer_validator_account != validators[author_index] {
+                    return Err(Error::<T>::WrongAuthority);
                 }
             }
         }

substrate-node/pallets/pallet-smart-contract/src/lib.rs

@@ -21,9 +21,12 @@ use pallet_tfgrid::{
     twin::TwinIp,
     DocumentHashInput, DocumentLinkInput, TwinIpInput,
 };
-use pallet_tfgrid::{CityNameInput, CountryNameInput, LatitudeInput, LongitudeInput, Ip4Input, Gw4Input};
+use pallet_tfgrid::{
+    CityNameInput, CountryNameInput, Gw4Input, Ip4Input, LatitudeInput, LongitudeInput,
+};
 use parking_lot::RwLock;
 use sp_core::{
+    crypto::key_types::DUMMY,
     crypto::Ss58Codec,
     offchain::{
         testing::{self},
@@ -32,16 +35,55 @@ use sp_core::{
     sr25519, Pair, Public, H256,
 };
 use sp_keystore::{testing::KeyStore, KeystoreExt, SyncCryptoStore};
-use sp_runtime::traits::{IdentifyAccount, Verify};
-use sp_runtime::{offchain::TransactionPool, MultiSignature};
 use sp_runtime::{
-    testing::{Header, TestXt},
-    traits::{BlakeTwo256, Extrinsic as ExtrinsicT, IdentityLookup},
-    AccountId32,
+    impl_opaque_keys,
+    offchain::TransactionPool,
+    testing::{Header, TestXt, UintAuthorityId},
+    traits::{
+        BlakeTwo256, Extrinsic as ExtrinsicT, IdentifyAccount, IdentityLookup, OpaqueKeys, Verify,
+    },
+    AccountId32, MultiSignature,
 };
 use sp_std::convert::{TryFrom, TryInto};
-use tfchain_support::traits::ChangeNode;
+use std::cell::RefCell;
+use tfchain_support::{constants::time::MINUTES, traits::ChangeNode};
+
+impl_opaque_keys! {
+    pub struct MockSessionKeys {
+        pub dummy: UintAuthorityId,
+    }
+}
+
+impl From<UintAuthorityId> for MockSessionKeys {
+    fn from(dummy: UintAuthorityId) -> Self {
+        Self { dummy }
+    }
+}
 
+pub const KEY_ID_A: KeyTypeId = KeyTypeId([4; 4]);
+pub const KEY_ID_B: KeyTypeId = KeyTypeId([9; 4]);
+
+#[derive(Debug, Clone, codec::Encode, codec::Decode, PartialEq, Eq)]
+pub struct PreUpgradeMockSessionKeys {
+    pub a: [u8; 32],
+    pub b: [u8; 64],
+}
+
+impl OpaqueKeys for PreUpgradeMockSessionKeys {
+    type KeyTypeIdProviders = ();
+
+    fn key_ids() -> &'static [KeyTypeId] {
+        &[KEY_ID_A, KEY_ID_B]
+    }
+
+    fn get_raw(&self, i: KeyTypeId) -> &[u8] {
+        match i {
+            i if i == KEY_ID_A => &self.a[..],
+            i if i == KEY_ID_B => &self.b[..],
+            _ => &[],
+        }
+    }
+}
 // set environment variable RUST_LOG=debug to see all logs when running the tests and call
 // env_logger::init() at the beginning of the test
 use env_logger;
@@ -68,6 +110,8 @@ construct_runtime!(
         SmartContractModule: pallet_smart_contract::{Pallet, Call, Storage, Event<T>},
         TFTPriceModule: pallet_tft_price::{Pallet, Call, Storage, Event<T>},
         Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent},
+        ValidatorSet: substrate_validator_set::{Pallet, Call, Storage, Event<T>, Config<T>},
+        Session: pallet_session::{Pallet, Call, Storage, Event, Config<T>},
     }
 );
 
@@ -246,6 +290,72 @@ impl pallet_authorship::Config for TestRuntime {
     type EventHandler = ();
 }
 
+parameter_types! {
+    pub const Period: u32 = 60 * MINUTES;
+    pub const Offset: u32 = 0;
+}
+
+thread_local! {
+    pub static VALIDATORS: RefCell<Vec<u64>> = RefCell::new(vec![1, 2, 3]);
+    pub static NEXT_VALIDATORS: RefCell<Vec<u64>> = RefCell::new(vec![1, 2, 3]);
+    pub static AUTHORITIES: RefCell<Vec<UintAuthorityId>> =
+        RefCell::new(vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]);
+    pub static FORCE_SESSION_END: RefCell<bool> = RefCell::new(false);
+    pub static SESSION_LENGTH: RefCell<u64> = RefCell::new(2);
+    pub static SESSION_CHANGED: RefCell<bool> = RefCell::new(false);
+    pub static DISABLED: RefCell<bool> = RefCell::new(false);
+    pub static BEFORE_SESSION_END_CALLED: RefCell<bool> = RefCell::new(false);
+}
+
+use pallet_session::SessionHandler;
+use sp_runtime::RuntimeAppPublic;
+pub struct TestSessionHandler;
+impl SessionHandler<AccountId> for TestSessionHandler {
+    const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[UintAuthorityId::ID];
+    fn on_genesis_session<T: OpaqueKeys>(_validators: &[(AccountId, T)]) {}
+    fn on_new_session<T: OpaqueKeys>(
+        changed: bool,
+        validators: &[(AccountId, T)],
+        _queued_validators: &[(AccountId, T)],
+    ) {
+        SESSION_CHANGED.with(|l| *l.borrow_mut() = changed);
+        AUTHORITIES.with(|l| {
+            *l.borrow_mut() = validators
+                .iter()
+                .map(|(_, id)| id.get::<UintAuthorityId>(DUMMY).unwrap_or_default())
+                .collect()
+        });
+    }
+    fn on_disabled(_validator_index: u32) {
+        DISABLED.with(|l| *l.borrow_mut() = true)
+    }
+    fn on_before_session_ending() {
+        BEFORE_SESSION_END_CALLED.with(|b| *b.borrow_mut() = true);
+    }
+}
+
+parameter_types! {
+    pub const MinAuthorities: u32 = 2;
+}
+
+impl substrate_validator_set::Config for TestRuntime {
+    type AddRemoveOrigin = EnsureRoot<Self::AccountId>;
+    type Event = Event;
+    type MinAuthorities = MinAuthorities;
+}
+
+impl pallet_session::Config for TestRuntime {
+    type Event = Event;
+    type ValidatorId = <Self as frame_system::Config>::AccountId;
+    type ValidatorIdOf = substrate_validator_set::ValidatorOf<Self>;
+    type ShouldEndSession = pallet_session::PeriodicSessions<Period, Offset>;
+    type NextSessionRotation = pallet_session::PeriodicSessions<Period, Offset>;
+    type SessionManager = ();
+    type SessionHandler = TestSessionHandler;
+    type Keys = MockSessionKeys;
+    type WeightInfo = ();
+}
+
 type AccountPublic = <MultiSignature as Verify>::Signer;
 
 pub(crate) fn get_name_contract_name(contract_name_input: &[u8]) -> TestNameContractName {