Add Cull Notices, Bump to m8

This patch adds "Cull notices" which culls any notice holds if they've already been executed. This is really meant to be a runtime upgrade fix, but that wasn't working, so this is just a temporary solution to make a runtime upgrade fix for test-net. We can remove the function in m9.

We also now always cull a notice hold if it's been executed.

integration/__tests__/upgrade_to_m8_scen.js

@@ -0,0 +1,65 @@
+const {
+  buildScenarios
+} = require('../util/scenario');
+const { decodeCall, getEventData } = require('../util/substrate');
+const { bytes32 } = require('../util/util');
+const { getNotice } = require('../util/substrate');
+
+let scen_info = {};
+
+buildScenarios('Upgrade to m8', scen_info, [
+  {
+    name: "Upgrade from m7 to m8",
+    info: {
+      versions: ['m7'],
+      genesis_version: 'm7',
+      eth_opts: {
+        version: 'm7',
+      },
+      validators: {
+        alice: {
+          version: 'm7',
+        },
+        bob: {
+          version: 'm7',
+        },
+        charlie: {
+          version: 'm7',
+          eth_private_key: "0000000000000000000000000000000000000000000000000000000000000001" // Bad key
+        }
+      },
+    },
+    scenario: async ({ ctx, chain, validators, starport, curr, sleep }) => {
+      const alice = validators.validatorInfoMap.alice;
+      const bob = validators.validatorInfoMap.bob;
+      const newAuthsRaw = [
+        { substrate_id: ctx.actors.keyring.decodeAddress(alice.aura_key), eth_address: alice.eth_account },
+        { substrate_id: ctx.actors.keyring.decodeAddress(bob.aura_key), eth_address: bob.eth_account }
+      ];
+
+      // Just set validators to same, but Bob won't be able to sign it
+      let extrinsic = ctx.api().tx.cash.changeValidators(newAuthsRaw);
+
+      let { notice } = await starport.executeProposal("Update authorities", [extrinsic], { awaitNotice: true });
+      await chain.waitUntilSession(1);
+
+      expect(await chain.noticeHold('Eth')).toEqual([1, 0]);
+
+      let signatures = await chain.getNoticeSignatures(notice, { signatures: 2 });
+      await starport.invoke(notice, signatures);
+      await sleep(10000);
+
+      expect(await chain.noticeState(notice)).toEqual({"Executed": null});
+      expect(await chain.noticeHold('Eth')).toEqual([1, 0]);
+
+      // Okay great, we've executed the change-over, but we still have a notice hold...
+      // But what if we upgrade to m8??
+      await chain.upgradeTo(curr);
+      await chain.cullNotices();
+      expect(await chain.noticeHold('Eth')).toEqual(null);
+
+      // start at 0, rotate through 1, actually perform change over on 2
+      await chain.waitUntilSession(2);
+    }
+  }
+]);

integration/util/scenario/chain.js

@@ -186,6 +186,20 @@ class Chain {
     return asset.unwrap().rate_model.toJSON();
   }
 
+  async noticeHold(chainId) {
+    return (await this.api().query.cash.noticeHolds(chainId)).toJSON();
+  }
+
+  async noticeState(notice) {
+    let chainId = getNoticeChainId(notice);
+    let noticeState = await this.api().query.cash.noticeStates(chainId, notice.NoticeId);
+    return noticeState.toJSON();
+  }
+
+  async cullNotices() {
+    return await sendAndWaitForEvents(this.api().tx.cash.cullNotices(), this.api());
+  }
+
   async nextCodeHash() {
     return mapToJson(await this.ctx.api().query.cash.allowedNextCodeHash());
   }

integration/util/scenario/validator.js

@@ -59,7 +59,7 @@ let validatorInfoMap = {
     eth_private_key: "50f05592dc31bfc65a77c4cc80f2764ba8f9a7cce29c94a51fe2d70cb5599374",
     eth_account: "0x6a72a2f14577D9Cd0167801EFDd54a07B40d2b61",
     node_key: '0x0000000000000000000000000000000000000000000000000000000000000001',
-    peer_id: '12D3KooWEyoppNCUx8Yx66oV9fJnriXwCcXwDDUA2kj6vnc6iDEp', // I have _no idea_ how this is generated
+    peer_id: '12D3KooWEyoppNCUx8Yx66oV9fJnriXwCcXwDDUA2kj6vnc6iDEp', // I have _no idea_ how this is generated (I just run a node with the node key and grab it)
     spawn_args: ['--alice'],
     color: chalk.blue,
     validator: true,
@@ -74,6 +74,17 @@ let validatorInfoMap = {
     spawn_args: ['--bob'],
     color: chalk.green,
     validator: true,
+  },
+  'charlie': {
+    aura_key: "5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y",
+    grandpa_key: "5DbKjhNLpqX3zqZdNBc9BGb4fHU1cRBaDhJUskrvkwfraDi6",
+    eth_private_key: "46848fdbde39184417f511187ebc87e12e3087ac67c630e18836a6813110310d",
+    eth_account: "0x714fea791A402f28BFB43B07f6C9A70482A8cF90",
+    node_key: '0x0000000000000000000000000000000000000000000000000000000000000003',
+    peer_id: '12D3KooWSCufgHzV4fCwRijfH2k3abrpAJxTKxEvN1FDuRXA2U9x', // I have _no idea_ how this is generated
+    spawn_args: ['--charlie'],
+    color: chalk.orange,
+    validator: true,
   }
 };
 

pallets/cash/src/internal/notices.rs

@@ -181,6 +181,12 @@ pub fn handle_notice_invoked<T: Config>(
         Reason::NoticeHashMismatch
     );
     Notices::take(chain_id, notice_id);
+    if let Some(notice_hold_id) = NoticeHolds::get(chain_id) {
+        if notice_hold_id == notice_id {
+            log!("Removing notice hold as executed");
+            NoticeHolds::take(chain_id);
+        }
+    }
     NoticeStates::insert(chain_id, notice_id, NoticeState::Executed);
     Ok(())
 }
@@ -295,6 +301,46 @@ mod tests {
 
     #[test]
     fn test_handle_notice_invoked_proper_notice() {
+        new_test_ext().execute_with(|| {
+            let chain_id = ChainId::Eth;
+            let notice_id = NoticeId(5, 6);
+            let notice_hold_id = NoticeId(4, 6);
+            let notice_hash = ChainHash::Eth([1; 32]);
+            let notice = Notice::ExtractionNotice(ExtractionNotice::Eth {
+                id: NoticeId(80, 1),
+                parent: [3u8; 32],
+                asset: [1; 20],
+                amount: 100,
+                account: [2; 20],
+            });
+
+            NoticeHolds::insert(chain_id, notice_hold_id);
+            NoticeHashes::insert(notice_hash, notice_id);
+            Notices::insert(chain_id, notice_id, notice);
+            NoticeStates::insert(
+                chain_id,
+                notice_id,
+                NoticeState::Pending {
+                    signature_pairs: ChainSignatureList::Eth(vec![]),
+                },
+            );
+
+            let result = handle_notice_invoked::<Test>(chain_id, notice_id, notice_hash, vec![]);
+
+            assert_eq!(result, Ok(()));
+
+            assert_eq!(NoticeHashes::get(notice_hash), Some(notice_id));
+            assert_eq!(Notices::get(chain_id, notice_id), None);
+            assert_eq!(
+                NoticeStates::get(chain_id, notice_id),
+                NoticeState::Executed
+            );
+            assert_eq!(NoticeHolds::get(chain_id), Some(notice_hold_id));
+        });
+    }
+
+    #[test]
+    fn test_handle_notice_invoked_proper_notice_removes_notice_hold() {
         new_test_ext().execute_with(|| {
             let chain_id = ChainId::Eth;
             let notice_id = NoticeId(5, 6);
@@ -307,6 +353,7 @@ mod tests {
                 account: [2; 20],
             });
 
+            NoticeHolds::insert(chain_id, notice_id);
             NoticeHashes::insert(notice_hash, notice_id);
             Notices::insert(chain_id, notice_id, notice);
             NoticeStates::insert(
@@ -327,6 +374,7 @@ mod tests {
                 NoticeStates::get(chain_id, notice_id),
                 NoticeState::Executed
             );
+            assert_eq!(NoticeHolds::get(chain_id), None);
         });
     }
 

pallets/cash/src/internal/validate_trx.rs

@@ -103,6 +103,11 @@ pub fn validate_unsigned<T: Config>(
                 Err(ValidationError::InvalidValidator)
             }
         }
+        Call::cull_notices() => Ok(ValidTransaction::with_tag_prefix("Gateway::cull_notices")
+            .priority(100)
+            .and_provides("cull_notices")
+            .propagate(false)
+            .build()),
         _ => Err(ValidationError::InvalidCall),
     }
 }

pallets/cash/src/lib.rs

@@ -29,10 +29,11 @@ use codec::alloc::string::String;
 use frame_support::{
     decl_event, decl_module, decl_storage, dispatch,
     sp_runtime::traits::Convert,
-    traits::{StoredMap, UnfilteredDispatchable},
+    traits::{OnRuntimeUpgrade, StoredMap, UnfilteredDispatchable},
     weights::{DispatchClass, GetDispatchInfo, Pays, Weight},
     Parameter,
 };
+use frame_system;
 use frame_system::{ensure_none, ensure_root, offchain::CreateSignedTransaction};
 use our_std::{collections::btree_set::BTreeSet, error, log, str, vec::Vec, Debuggable};
 use sp_core::crypto::AccountId32;
@@ -575,6 +576,22 @@ decl_module! {
             ensure_none(origin)?;
             Ok(check_failure::<T>(internal::exec_trx_request::exec::<T>(request, signature, nonce))?)
         }
+
+        // Remove any notice holds if they have been executed
+        #[weight = (1, DispatchClass::Normal, Pays::No)] // XXX
+        pub fn cull_notices(origin) -> dispatch::DispatchResult {
+            log!("Culling executed notices");
+            NoticeHolds::iter().for_each(|(chain_id, notice_id)| {
+                match NoticeStates::get(chain_id, notice_id) {
+                    NoticeState::Executed => {
+                        NoticeHolds::take(chain_id);
+                        log!("Removed notice hold {:?}:{:?} as it was already executed", chain_id, notice_id);
+                    },
+                    _ => ()
+                }
+            });
+            Ok(())
+        }
     }
 }
 

runtime/src/lib.rs

@@ -121,7 +121,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("gateway"),
     impl_name: create_runtime_str!("gateway"),
     authoring_version: 1,
-    spec_version: 7,
+    spec_version: 8,
     impl_version: 1,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 1,