mirror of
https://github.com/serai-dex/serai.git
synced 2025-12-08 20:29:23 +00:00
Move in instructions from inherent transactions to unsigned transactions
The original intent was to use inherent transactions to prevent needing to vote on-chain, which would spam the chain with worthless votes. Inherent transactions, and our Tendermint library, would use the BFT's processs voting to also vote on all included transactions. This perfectly collapses integrity voting creating *no additional on-chain costs*. Unfortunately, this led to issues such as #6, along with questions of validator scalability when all validators are expencted to participate in consensus (in order to vote on if the included instructions are valid). This has been summarized in #241. With this change, we can remove Tendermint from Substrate. This greatly decreases our complexity. While I'm unhappy with the amount of time spent on it, just to reach this conclusion, thankfully tendermint-machine itself is still usable for #163. This also has reached a tipping point recently as the polkadot-v0.9.40 branch of substrate changed how syncing works, requiring further changes to sc-tendermint. These have no value if we're just going to get rid of it later, due to fundamental design issues, yet I would like to keep Substrate updated. This should be followed by moving back to GRANDPA, enabling closing most open Tendermint issues. Please note the current in-instructions-pallet does not actually verify the included signature yet. It's marked TODO, despite this bing critical.
This commit is contained in:
Luke Parker
@@ -4,60 +4,20 @@
|
||||
|
||||
use scale::{Encode, Decode};
|
||||
|
||||
use sp_inherents::{InherentIdentifier, IsFatalError};
|
||||
|
||||
use sp_runtime::RuntimeDebug;
|
||||
|
||||
use serai_primitives::{BlockNumber, BlockHash, Coin, WithAmount, Balance};
|
||||
use serai_primitives::{BlockHash, NetworkId};
|
||||
|
||||
pub use in_instructions_primitives as primitives;
|
||||
use primitives::{InInstruction, Updates};
|
||||
|
||||
pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"ininstrs";
|
||||
use primitives::{InInstruction, InInstructionWithBalance, SignedBatch};
|
||||
|
||||
#[derive(Clone, Copy, Encode, RuntimeDebug)]
|
||||
#[cfg_attr(feature = "std", derive(Decode, thiserror::Error))]
|
||||
pub enum InherentError {
|
||||
#[cfg_attr(feature = "std", error("invalid call"))]
|
||||
InvalidCall,
|
||||
#[cfg_attr(feature = "std", error("inherent has {0} updates despite us having {1} coins"))]
|
||||
InvalidUpdateQuantity(u32, u32),
|
||||
#[cfg_attr(
|
||||
feature = "std",
|
||||
error("inherent for coin {0:?} has block number {1:?} despite us having {2:?}")
|
||||
)]
|
||||
UnrecognizedBlockNumber(Coin, BlockNumber, BlockNumber),
|
||||
#[cfg_attr(
|
||||
feature = "std",
|
||||
error("inherent for coin {0:?} has block number {1:?} which doesn't succeed {2:?}")
|
||||
)]
|
||||
InvalidBlockNumber(Coin, BlockNumber, BlockNumber),
|
||||
#[cfg_attr(feature = "std", error("coin {0:?} has {1} more batches than we do"))]
|
||||
UnrecognizedBatches(Coin, u32),
|
||||
#[cfg_attr(feature = "std", error("coin {0:?} has a different batch (ID {1:?})"))]
|
||||
DifferentBatch(Coin, BlockHash),
|
||||
}
|
||||
|
||||
impl IsFatalError for InherentError {
|
||||
fn is_fatal_error(&self) -> bool {
|
||||
match self {
|
||||
InherentError::InvalidCall | InherentError::InvalidUpdateQuantity(..) => true,
|
||||
InherentError::UnrecognizedBlockNumber(..) => false,
|
||||
InherentError::InvalidBlockNumber(..) => true,
|
||||
InherentError::UnrecognizedBatches(..) => false,
|
||||
// One of our nodes is definitively wrong. If it's ours (signified by it passing consensus),
|
||||
// we should panic. If it's theirs, they should be slashed
|
||||
// Unfortunately, we can't return fatal here to trigger a slash as fatal should only be used
|
||||
// for undeniable, technical invalidity
|
||||
// TODO: Code a way in which this still triggers a slash vote
|
||||
InherentError::DifferentBatch(..) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn coin_from_index(index: usize) -> Coin {
|
||||
// Offset by 1 since Serai is the first coin, yet Serai doesn't have updates
|
||||
Coin::from(1 + u32::try_from(index).unwrap())
|
||||
pub enum PalletError {
|
||||
#[cfg_attr(feature = "std", error("batch for unrecognized network"))]
|
||||
UnrecognizedNetwork,
|
||||
#[cfg_attr(feature = "std", error("invalid signature for batch"))]
|
||||
InvalidSignature,
|
||||
}
|
||||
|
||||
#[frame_support::pallet]
|
||||
@@ -77,36 +37,23 @@ pub mod pallet {
|
||||
#[pallet::event]
|
||||
#[pallet::generate_deposit(fn deposit_event)]
|
||||
pub enum Event<T: Config> {
|
||||
Batch { coin: Coin, id: BlockHash },
|
||||
Failure { coin: Coin, id: BlockHash, index: u32 },
|
||||
Batch { network: NetworkId, id: u32, block: BlockHash },
|
||||
InstructionFailure { network: NetworkId, id: u32, index: u32 },
|
||||
}
|
||||
|
||||
#[pallet::pallet]
|
||||
#[pallet::generate_store(pub(crate) trait Store)]
|
||||
pub struct Pallet<T>(PhantomData<T>);
|
||||
|
||||
// Used to only allow one set of updates per block, preventing double updating
|
||||
#[pallet::storage]
|
||||
pub(crate) type Once<T: Config> = StorageValue<_, bool, ValueQuery>;
|
||||
// Latest block number agreed upon for a coin
|
||||
#[pallet::storage]
|
||||
#[pallet::getter(fn block_number)]
|
||||
pub(crate) type BlockNumbers<T: Config> =
|
||||
StorageMap<_, Blake2_256, Coin, BlockNumber, ValueQuery>;
|
||||
|
||||
#[pallet::hooks]
|
||||
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
|
||||
fn on_finalize(_: BlockNumberFor<T>) {
|
||||
Once::<T>::take();
|
||||
}
|
||||
}
|
||||
#[pallet::getter(fn batch)]
|
||||
pub(crate) type Batches<T: Config> = StorageMap<_, Blake2_256, NetworkId, u32, OptionQuery>;
|
||||
|
||||
impl<T: Config> Pallet<T> {
|
||||
fn execute(coin: Coin, instruction: WithAmount<InInstruction>) -> Result<(), ()> {
|
||||
match instruction.data {
|
||||
InInstruction::Transfer(address) => {
|
||||
Tokens::<T>::mint(address, Balance { coin, amount: instruction.amount })
|
||||
}
|
||||
fn execute(instruction: InInstructionWithBalance) -> Result<(), ()> {
|
||||
match instruction.instruction {
|
||||
InInstruction::Transfer(address) => Tokens::<T>::mint(address, instruction.balance),
|
||||
_ => panic!("unsupported instruction"),
|
||||
}
|
||||
Ok(())
|
||||
@@ -117,28 +64,24 @@ pub mod pallet {
|
||||
impl<T: Config> Pallet<T> {
|
||||
#[pallet::call_index(0)]
|
||||
#[pallet::weight((0, DispatchClass::Operational))] // TODO
|
||||
pub fn update(origin: OriginFor<T>, mut updates: Updates) -> DispatchResult {
|
||||
pub fn execute_batch(origin: OriginFor<T>, batch: SignedBatch) -> DispatchResult {
|
||||
ensure_none(origin)?;
|
||||
assert!(!Once::<T>::exists());
|
||||
Once::<T>::put(true);
|
||||
|
||||
for (coin, update) in updates.iter_mut().enumerate() {
|
||||
if let Some(update) = update {
|
||||
let coin = coin_from_index(coin);
|
||||
BlockNumbers::<T>::insert(coin, update.block_number);
|
||||
let mut batch = batch.batch;
|
||||
|
||||
for batch in update.batches.iter_mut() {
|
||||
Self::deposit_event(Event::Batch { coin, id: batch.id });
|
||||
for (i, instruction) in batch.instructions.drain(..).enumerate() {
|
||||
if Self::execute(coin, instruction).is_err() {
|
||||
Self::deposit_event(Event::Failure {
|
||||
coin,
|
||||
id: batch.id,
|
||||
index: u32::try_from(i).unwrap(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
Batches::<T>::insert(batch.network, batch.id);
|
||||
Self::deposit_event(Event::Batch {
|
||||
network: batch.network,
|
||||
id: batch.id,
|
||||
block: batch.block,
|
||||
});
|
||||
for (i, instruction) in batch.instructions.drain(..).enumerate() {
|
||||
if Self::execute(instruction).is_err() {
|
||||
Self::deposit_event(Event::InstructionFailure {
|
||||
network: batch.network,
|
||||
id: batch.id,
|
||||
index: u32::try_from(i).unwrap(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -146,92 +89,40 @@ pub mod pallet {
|
||||
}
|
||||
}
|
||||
|
||||
#[pallet::inherent]
|
||||
impl<T: Config> ProvideInherent for Pallet<T> {
|
||||
#[pallet::validate_unsigned]
|
||||
impl<T: Config> ValidateUnsigned for Pallet<T> {
|
||||
type Call = Call<T>;
|
||||
type Error = InherentError;
|
||||
const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;
|
||||
|
||||
fn create_inherent(data: &InherentData) -> Option<Self::Call> {
|
||||
data
|
||||
.get_data::<Updates>(&INHERENT_IDENTIFIER)
|
||||
.unwrap()
|
||||
.map(|updates| Call::update { updates })
|
||||
}
|
||||
|
||||
// Assumes that only not yet handled batches are provided as inherent data
|
||||
fn check_inherent(call: &Self::Call, data: &InherentData) -> Result<(), Self::Error> {
|
||||
// First unwrap is for the Result of fetching/decoding the Updates
|
||||
// Second unwrap is for the Option of if they exist
|
||||
let expected = data.get_data::<Updates>(&INHERENT_IDENTIFIER).unwrap().unwrap();
|
||||
fn validate_unsigned(_: TransactionSource, call: &Self::Call) -> TransactionValidity {
|
||||
// Match to be exhaustive
|
||||
let updates = match call {
|
||||
Call::update { ref updates } => updates,
|
||||
_ => Err(InherentError::InvalidCall)?,
|
||||
let batch = match call {
|
||||
Call::execute_batch { ref batch } => batch,
|
||||
_ => Err(InvalidTransaction::Call)?,
|
||||
};
|
||||
|
||||
// The block producer should've provided one update per coin
|
||||
// We, an honest node, did provide one update per coin
|
||||
// Accordingly, we should have the same amount of updates
|
||||
if updates.len() != expected.len() {
|
||||
Err(InherentError::InvalidUpdateQuantity(
|
||||
updates.len().try_into().unwrap(),
|
||||
expected.len().try_into().unwrap(),
|
||||
))?;
|
||||
let network = batch.batch.network;
|
||||
|
||||
// TODO: Get the key for this network or Err(UnrecognizedNetwork)
|
||||
|
||||
// TODO: Verify the signature or Err(InvalidSignature)
|
||||
|
||||
// Verify the batch is sequential
|
||||
// Batches has the last ID set. The next ID should be it + 1
|
||||
// If there's no ID, the next ID should be 0
|
||||
let expected = Batches::<T>::get(network).map(|prev| prev + 1).unwrap_or(0);
|
||||
if batch.batch.id < expected {
|
||||
Err(InvalidTransaction::Stale)?;
|
||||
}
|
||||
if batch.batch.id > expected {
|
||||
Err(InvalidTransaction::Future)?;
|
||||
}
|
||||
|
||||
// This zip is safe since we verified they're equally sized
|
||||
// This should be written as coins.zip(updates.iter().zip(&expected)), where coins is the
|
||||
// validator set's coins
|
||||
// That'd require having context on the validator set right now which isn't worth pulling in
|
||||
// right now, when we only have one validator set
|
||||
for (coin, both) in updates.iter().zip(&expected).enumerate() {
|
||||
let coin = coin_from_index(coin);
|
||||
match both {
|
||||
// Block producer claims there's an update for this coin, as do we
|
||||
(Some(update), Some(expected)) => {
|
||||
if update.block_number.0 > expected.block_number.0 {
|
||||
Err(InherentError::UnrecognizedBlockNumber(
|
||||
coin,
|
||||
update.block_number,
|
||||
expected.block_number,
|
||||
))?;
|
||||
}
|
||||
|
||||
let prev = BlockNumbers::<T>::get(coin);
|
||||
if update.block_number.0 <= prev.0 {
|
||||
Err(InherentError::InvalidBlockNumber(coin, update.block_number, prev))?;
|
||||
}
|
||||
|
||||
if update.batches.len() > expected.batches.len() {
|
||||
Err(InherentError::UnrecognizedBatches(
|
||||
coin,
|
||||
(update.batches.len() - expected.batches.len()).try_into().unwrap(),
|
||||
))?;
|
||||
}
|
||||
|
||||
for (batch, expected) in update.batches.iter().zip(&expected.batches) {
|
||||
if batch != expected {
|
||||
Err(InherentError::DifferentBatch(coin, batch.id))?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Block producer claims there's an update for this coin, yet we don't
|
||||
(Some(update), None) => {
|
||||
Err(InherentError::UnrecognizedBatches(coin, update.batches.len().try_into().unwrap()))?
|
||||
}
|
||||
|
||||
// Block producer didn't include update for this coin
|
||||
(None, _) => (),
|
||||
};
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_inherent(_: &Self::Call) -> bool {
|
||||
true
|
||||
ValidTransaction::with_tag_prefix("in-instructions")
|
||||
.and_provides((batch.batch.network, batch.batch.id))
|
||||
// Set a 10 block longevity, though this should be included in the next block
|
||||
.longevity(10)
|
||||
.propagate(true)
|
||||
.build()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user