Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GMP precompile killswitch #2292

Merged
merged 12 commits into from
May 22, 2023
40 changes: 36 additions & 4 deletions precompiles/gmp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
#![cfg_attr(not(feature = "std"), no_std)]

use evm::ExitReason;
use fp_evm::{Context, PrecompileFailure, PrecompileHandle};
use fp_evm::{Context, ExitRevert, PrecompileFailure, PrecompileHandle};
use frame_support::{
codec::Decode,
dispatch::{Dispatchable, GetDispatchInfo, PostDispatchInfo},
traits::ConstU32,
};
use pallet_evm::AddressMapping;
use parity_scale_codec::DecodeLimit;
use precompile_utils::prelude::*;
use precompile_utils::{prelude::*, solidity::revert::revert_as_bytes};
use sp_core::{H160, U256};
use sp_std::boxed::Box;
use sp_std::{marker::PhantomData, vec::Vec};
Expand Down Expand Up @@ -78,12 +78,14 @@ where
log::debug!(target: "gmp-precompile", "wormhole_vaa: {:?}", wormhole_vaa.clone());

// tally up gas cost:
// 1 read for enabled flag
// 2 reads for contract addresses
// 2500 as fudge for computation, esp. payload decoding (TODO: benchmark?)
let initial_gas = 2500 + 2 * RuntimeHelper::<Runtime>::db_read_gas_cost();
log::warn!("initial_gas: {:?}", initial_gas);
let initial_gas = 2500 + 3 * RuntimeHelper::<Runtime>::db_read_gas_cost();
handle.record_cost(initial_gas)?;

ensure_enabled()?;

let wormhole = storage::CoreAddress::get()
.ok_or(RevertReason::custom("invalid wormhole core address"))?;

Expand Down Expand Up @@ -242,11 +244,30 @@ fn ensure_exit_reason_success(reason: ExitReason, output: &[u8]) -> EvmResult<()
}
}

pub fn is_enabled() -> bool {
match storage::PrecompileEnabled::get() {
Some(enabled) => enabled,
_ => false,
}
}

fn ensure_enabled() -> EvmResult<()> {
if is_enabled() {
Ok(())
} else {
Err(PrecompileFailure::Revert {
exit_status: ExitRevert::Reverted,
output: revert_as_bytes("GMP Precompile is not enabled"),
})
}
}

/// We use pallet storage in our precompile by implementing a StorageInstance for each item we need
/// to store.
/// twox_128("gmp") => 0xb7f047395bba5df0367b45771c00de50
/// twox_128("CoreAddress") => 0x59ff23ff65cc809711800d9d04e4b14c
/// twox_128("BridgeAddress") => 0xc1586bde54b249fb7f521faf831ade45
/// twox_128("PrecompileEnabled") => 0x2551bba17abb82ef3498bab688e470b8
mod storage {
use super::*;
use frame_support::{
Expand All @@ -273,4 +294,15 @@ mod storage {
}
}
pub type BridgeAddress = StorageValue<BridgeAddressStorageInstance, H160, OptionQuery>;

// storage for precompile enabled
// None or Some(false) both mean that the precompile is disabled; only Some(true) means enabled.
pub struct PrecompileEnabledStorageInstance;
impl StorageInstance for PrecompileEnabledStorageInstance {
const STORAGE_PREFIX: &'static str = "PrecompileEnabled";
fn pallet_prefix() -> &'static str {
"gmp"
}
}
pub type PrecompileEnabled = StorageValue<PrecompileEnabledStorageInstance, bool, OptionQuery>;
}
38 changes: 38 additions & 0 deletions precompiles/gmp/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,3 +385,41 @@ impl orml_xtokens::Config for Runtime {
type ReserveProvider = AbsoluteReserveProvider;
type UniversalLocation = UniversalLocation;
}

pub(crate) struct ExtBuilder {
/// Endowed accounts with balances
balances: Vec<(AccountId, Balance)>,
}

impl Default for ExtBuilder {
fn default() -> ExtBuilder {
ExtBuilder { balances: vec![] }
}
}

impl ExtBuilder {
/// Fund some accounts before starting the test
pub(crate) fn with_balances(mut self, balances: Vec<(AccountId, Balance)>) -> Self {
self.balances = balances;
self
}

/// Build the test externalities for use in tests
pub(crate) fn build(self) -> sp_io::TestExternalities {
let mut t = frame_system::GenesisConfig::default()
.build_storage::<Runtime>()
.expect("Frame system builds valid default genesis config");

pallet_balances::GenesisConfig::<Runtime> {
balances: self.balances.clone(),
}
.assimilate_storage(&mut t)
.expect("Pallet balances storage can be assimilated");

let mut ext = sp_io::TestExternalities::new(t);
ext.execute_with(|| {
System::set_block_number(1);
});
ext
}
}
91 changes: 89 additions & 2 deletions precompiles/gmp/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,95 @@
// You should have received a copy of the GNU General Public License
// along with Moonbeam. If not, see <http://www.gnu.org/licenses/>.

use crate::mock::PCall;
use precompile_utils::testing::*;
use crate::mock::*;
use fp_evm::{ExitRevert, PrecompileFailure};
use precompile_utils::{solidity::revert::revert_as_bytes, testing::*};

fn precompiles() -> Precompiles<Runtime> {
PrecompilesValue::get()
}

#[test]
fn contract_disabling_default_value_is_false() {
ExtBuilder::default()
.with_balances(vec![(Alice.into(), 100_000)])
.build()
.execute_with(|| {
// default should be false
assert_eq!(crate::storage::PrecompileEnabled::get(), None);
assert_eq!(crate::is_enabled(), false);
assert_eq!(
crate::ensure_enabled(),
Err(PrecompileFailure::Revert {
exit_status: ExitRevert::Reverted,
output: revert_as_bytes("GMP Precompile is not enabled"),
})
);

precompiles()
.prepare_test(
CryptoAlith,
Precompile1,
PCall::wormhole_transfer_erc20 {
wormhole_vaa: Vec::new().into(),
},
)
.execute_reverts(|output| output == b"GMP Precompile is not enabled");
})
}

#[test]
fn contract_enabling_works() {
ExtBuilder::default()
.with_balances(vec![(Alice.into(), 100_000)])
.build()
.execute_with(|| {
crate::storage::PrecompileEnabled::set(Some(true));
assert_eq!(crate::storage::PrecompileEnabled::get(), Some(true));
assert_eq!(crate::is_enabled(), true);
assert_eq!(crate::ensure_enabled(), Ok(()));

// should fail at a later point since contract addresses are not set
precompiles()
.prepare_test(
CryptoAlith,
Precompile1,
PCall::wormhole_transfer_erc20 {
wormhole_vaa: Vec::new().into(),
},
)
.execute_reverts(|output| output == b"invalid wormhole core address");
})
}

#[test]
fn contract_disabling_works() {
ExtBuilder::default()
.with_balances(vec![(Alice.into(), 100_000)])
.build()
.execute_with(|| {
crate::storage::PrecompileEnabled::set(Some(false));
assert_eq!(crate::storage::PrecompileEnabled::get(), Some(false));
assert_eq!(crate::is_enabled(), false);
assert_eq!(
crate::ensure_enabled(),
Err(PrecompileFailure::Revert {
exit_status: ExitRevert::Reverted,
output: revert_as_bytes("GMP Precompile is not enabled"),
})
);

precompiles()
.prepare_test(
CryptoAlith,
Precompile1,
PCall::wormhole_transfer_erc20 {
wormhole_vaa: Vec::new().into(),
},
)
.execute_reverts(|output| output == b"GMP Precompile is not enabled");
})
}

#[test]
fn test_solidity_interface_has_all_function_selectors_documented_and_implemented() {
Expand Down
39 changes: 38 additions & 1 deletion tests/tests/test-precompile/test-precompile-wormhole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import { alith, ALITH_ADDRESS, ALITH_PRIVATE_KEY, BALTATHAR_ADDRESS } from "../.
import { PRECOMPILE_GMP_ADDRESS } from "../../util/constants";
import { expectSubstrateEvent, expectSubstrateEvents } from "../../util/expect";

import { expectEVMResult } from "../../util/eth-transactions";
import { expectEVMResult, extractRevertReason } from "../../util/eth-transactions";
import { expect } from "chai";
const debug = require("debug")("test:wormhole");

const GUARDIAN_SET_INDEX = 0;
Expand Down Expand Up @@ -209,6 +210,21 @@ describeDevMoonbeam(`Test local Wormhole`, (context) => {
.signAndSend(alith);
await context.createBlock();

// we also need to disable the killswitch by setting the 'enabled' flag to Some(true)
const ENABLED_FLAG_STORAGE_ADDRESS =
"0xb7f047395bba5df0367b45771c00de502551bba17abb82ef3498bab688e470b8";
Copy link
Contributor

@timbrinded timbrinded May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be tedious but can you derive this instead, so that it's clear what this is?

This is to help future generations having to work out where that key is from (and it's a pain to work backwards from a hash).

  const enabledFlagKey = u8aToHex(
            u8aConcat(xxhashAsU8a("gmp", 128), xxhashAsU8a("PrecompileEnabled", 128))
          );

ℹ️ both functions are available in the @polkadot/util repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. Added in 751af83. I also left the keys themselves for future generations 😁

await context.polkadotApi.tx.sudo
.sudo(
context.polkadotApi.tx.system.setStorage([
[
ENABLED_FLAG_STORAGE_ADDRESS,
context.polkadotApi.registry.createType("Option<bool>", true).toHex(),
],
])
)
.signAndSend(alith);
await context.createBlock();

const transferVAA = await genTransferWithPayloadVAA(
signerPKs,
GUARDIAN_SET_INDEX,
Expand Down Expand Up @@ -239,3 +255,24 @@ describeDevMoonbeam(`Test local Wormhole`, (context) => {
expectSubstrateEvents(result, "xTokens", "TransferredMultiAssets");
});
});

describeDevMoonbeam(`Test GMP Killswitch`, (context) => {
it("should fail with killswitch enabled by default", async function () {
// payload should be irrelevant since the precompile will fail before attempting to decode
const transferVAA = "deadbeef";

const data = GMP_INTERFACE.encodeFunctionData("wormholeTransferERC20", [`0x${transferVAA}`]);

const result = await context.createBlock(
createTransaction(context, {
to: PRECOMPILE_GMP_ADDRESS,
gas: 500_000,
data,
})
);

expectEVMResult(result.result.events, "Revert", "Reverted");
const revertReason = await extractRevertReason(result.result.hash, context.ethers);
expect(revertReason).to.contain("GMP Precompile is not enabled");
});
});