From aa6110ce06a5eb5da43b98c2cbee59234bc9b2b2 Mon Sep 17 00:00:00 2001 From: Geoffrey Hayes Date: Fri, 19 Mar 2021 01:13:38 -0700 Subject: [PATCH] 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 | 65 +++++++++++++++++++++ integration/util/scenario/chain.js | 14 +++++ integration/util/scenario/validator.js | 13 ++++- pallets/cash/src/internal/notices.rs | 48 +++++++++++++++ pallets/cash/src/internal/validate_trx.rs | 5 ++ pallets/cash/src/lib.rs | 19 +++++- runtime/src/lib.rs | 2 +- 7 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 integration/__tests__/upgrade_to_m8_scen.js diff --git a/integration/__tests__/upgrade_to_m8_scen.js b/integration/__tests__/upgrade_to_m8_scen.js new file mode 100644 index 000000000..8b3517870 --- /dev/null +++ b/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); + } + } +]); diff --git a/integration/util/scenario/chain.js b/integration/util/scenario/chain.js index 1de7ec2f6..6fe08131a 100644 --- a/integration/util/scenario/chain.js +++ b/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()); } diff --git a/integration/util/scenario/validator.js b/integration/util/scenario/validator.js index a429b960d..33ce62edb 100644 --- a/integration/util/scenario/validator.js +++ b/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, } }; diff --git a/pallets/cash/src/internal/notices.rs b/pallets/cash/src/internal/notices.rs index 89cdada45..be9f67ef1 100644 --- a/pallets/cash/src/internal/notices.rs +++ b/pallets/cash/src/internal/notices.rs @@ -181,6 +181,12 @@ pub fn handle_notice_invoked( 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::(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); }); } diff --git a/pallets/cash/src/internal/validate_trx.rs b/pallets/cash/src/internal/validate_trx.rs index fc1517599..06f43f1e4 100644 --- a/pallets/cash/src/internal/validate_trx.rs +++ b/pallets/cash/src/internal/validate_trx.rs @@ -103,6 +103,11 @@ pub fn validate_unsigned( 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), } } diff --git a/pallets/cash/src/lib.rs b/pallets/cash/src/lib.rs index 128d41017..427903289 100644 --- a/pallets/cash/src/lib.rs +++ b/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::(internal::exec_trx_request::exec::(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(()) + } } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 402d1851c..c60ba06aa 100644 --- a/runtime/src/lib.rs +++ b/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,