Skip to content

Commit

Permalink
refactor: move validation part of polyjuice creator id into Transacti…
Browse files Browse the repository at this point in the history
…onVerfier
  • Loading branch information
magicalne committed Aug 31, 2022
1 parent 354eb81 commit cd170ee
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 66 deletions.
1 change: 0 additions & 1 deletion crates/block-producer/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,6 @@ pub async fn run(config: Config, skip_config_check: bool) -> Result<()> {
node_mode: config.node_mode,
dynamic_config_manager: base.dynamic_config_manager.clone(),
has_p2p_sync: config.p2p_network_config.is_some(),
polyjuice_creator_id: None,
};
Arc::new(Mutex::new(
MemPool::create(args)
Expand Down
2 changes: 2 additions & 0 deletions crates/generator/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ pub enum TransactionError {
Utf8Error(std::str::Utf8Error),
#[error("Native token transfer error, invalide to_id: {0}")]
NativeTransferInvalidToId(u32),
#[error("Polyjuice creator id not found.")]
PolyjuiceCreatorIdNotFound,
}

impl From<VMError> for TransactionError {
Expand Down
64 changes: 39 additions & 25 deletions crates/generator/src/verification/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,42 @@
use std::sync::Arc;

use arc_swap::ArcSwapOption;
use gw_common::{
builtins::{CKB_SUDT_ACCOUNT_ID, ETH_REGISTRY_ACCOUNT_ID},
state::State,
};
use gw_traits::CodeStore;
use gw_types::{offchain::RollupContext, packed::L2Transaction, prelude::*};
use gw_types::{packed::L2Transaction, prelude::*};
use tracing::instrument;

use crate::{
constants::MAX_TX_SIZE,
error::{AccountError, TransactionError, TransactionValidateError},
typed_transaction::types::TypedRawTransaction,
utils::get_tx_type,
utils::{get_polyjuice_creator_id, get_tx_type},
Generator,
};

pub struct TransactionVerifier<'a, S> {
state: &'a S,
rollup_context: &'a RollupContext,
polyjuice_creator_id: Option<u32>,
pub struct TransactionVerifier {
generator: Arc<Generator>,
polyjuice_creator_id: ArcSwapOption<u32>,
}

impl<'a, S: State + CodeStore> TransactionVerifier<'a, S> {
pub fn new(
state: &'a S,
rollup_context: &'a RollupContext,
polyjuice_creator_id: Option<u32>,
) -> Self {
impl TransactionVerifier {
pub fn new(generator: Arc<Generator>) -> Self {
Self {
state,
rollup_context,
polyjuice_creator_id,
generator,
polyjuice_creator_id: ArcSwapOption::from(None),
}
}
/// verify transaction
/// Notice this function do not perform signature check
#[instrument(skip_all)]
pub fn verify(&self, tx: &L2Transaction) -> Result<(), TransactionValidateError> {
pub fn verify<S: State + CodeStore>(
&self,
state: &S,
tx: &L2Transaction,
) -> Result<(), TransactionValidateError> {
let raw_tx = tx.raw();
let sender_id: u32 = raw_tx.from_id().unpack();

Expand All @@ -48,7 +50,7 @@ impl<'a, S: State + CodeStore> TransactionVerifier<'a, S> {
}

// verify nonce
let account_nonce: u32 = self.state.get_nonce(sender_id)?;
let account_nonce: u32 = state.get_nonce(sender_id)?;
let nonce: u32 = raw_tx.nonce().unpack();
if nonce != account_nonce {
return Err(TransactionError::Nonce {
Expand All @@ -60,16 +62,13 @@ impl<'a, S: State + CodeStore> TransactionVerifier<'a, S> {
}

// verify balance
let sender_script_hash = self.state.get_script_hash(sender_id)?;
let sender_address = self
.state
let sender_script_hash = state.get_script_hash(sender_id)?;
let sender_address = state
.get_registry_address_by_script_hash(ETH_REGISTRY_ACCOUNT_ID, &sender_script_hash)?
.ok_or(AccountError::RegistryAddressNotFound)?;
// get balance
let balance = self
.state
.get_sudt_balance(CKB_SUDT_ACCOUNT_ID, &sender_address)?;
let tx_type = get_tx_type(self.rollup_context, self.state, &tx.raw())?;
let balance = state.get_sudt_balance(CKB_SUDT_ACCOUNT_ID, &sender_address)?;
let tx_type = get_tx_type(self.generator.rollup_context(), state, &tx.raw())?;
let typed_tx =
TypedRawTransaction::from_tx(tx.raw(), tx_type).ok_or(AccountError::UnknownScript)?;
// reject txs has no cost, these transaction can only be execute without modify state tree
Expand Down Expand Up @@ -103,7 +102,22 @@ impl<'a, S: State + CodeStore> TransactionVerifier<'a, S> {
if p.is_native_transfer() {
// Verify to_id is CREATOR_ID
let to_id = raw_tx.to_id().unpack();
if Some(to_id) != self.polyjuice_creator_id && self.polyjuice_creator_id.is_some() {
if self.polyjuice_creator_id.load_full().is_some() {
let polyjuice_creator_id = get_polyjuice_creator_id(
self.generator.rollup_context(),
self.generator.backend_manage(),
state,
)?;
let polyjuice_creator_id =
polyjuice_creator_id.ok_or(TransactionError::PolyjuiceCreatorIdNotFound)?;
self.polyjuice_creator_id
.store(Some(Arc::new(polyjuice_creator_id)));
}
let creator_id = self
.polyjuice_creator_id
.load_full()
.map(|creator_id| *creator_id);
if Some(to_id) != creator_id {
return Err(TransactionError::NativeTransferInvalidToId(to_id).into());
}
}
Expand Down
26 changes: 4 additions & 22 deletions crates/mem-pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use gw_generator::{
error::TransactionError,
generator::CyclesPool,
traits::StateExt,
utils::get_polyjuice_creator_id,
verification::{transaction::TransactionVerifier, withdrawal::WithdrawalVerifier},
ArcSwap, Generator,
};
Expand Down Expand Up @@ -108,7 +107,7 @@ pub struct MemPool {
has_p2p_sync: bool,
/// Cycles Pool
cycles_pool: CyclesPool,
polyjuice_creator_id: Option<u32>,
tx_verifier: TransactionVerifier,
}

pub struct MemPoolCreateArgs {
Expand All @@ -120,7 +119,6 @@ pub struct MemPoolCreateArgs {
pub node_mode: NodeMode,
pub dynamic_config_manager: Arc<ArcSwap<DynamicConfigManager>>,
pub has_p2p_sync: bool,
pub polyjuice_creator_id: Option<u32>,
}

impl Drop for MemPool {
Expand All @@ -144,7 +142,6 @@ impl MemPool {
node_mode,
dynamic_config_manager,
has_p2p_sync,
polyjuice_creator_id,
} = args;
let pending = Default::default();

Expand Down Expand Up @@ -196,6 +193,7 @@ impl MemPool {
config.mem_block.syscall_cycles.clone(),
);

let tx_verifier = TransactionVerifier::new(generator.clone());
let mut mem_pool = MemPool {
store,
current_tip: tip,
Expand All @@ -214,7 +212,7 @@ impl MemPool {
mem_block_config: config.mem_block,
has_p2p_sync,
cycles_pool,
polyjuice_creator_id,
tx_verifier,
};
mem_pool.restore_pending_withdrawals().await?;

Expand Down Expand Up @@ -331,9 +329,7 @@ impl MemPool {
}

// verify transaction
let polyjuice_creator_id = self.get_polyjuice_creator_id()?;
TransactionVerifier::new(state, self.generator.rollup_context(), polyjuice_creator_id)
.verify(&tx)?;
self.tx_verifier.verify(state, &tx)?;
// verify signature
self.generator.check_transaction_signature(state, &tx)?;

Expand Down Expand Up @@ -461,20 +457,6 @@ impl MemPool {
Self::package_mem_block(&self.mem_block, output_param)
}

fn get_polyjuice_creator_id(&mut self) -> Result<Option<u32>> {
let snap = self.mem_pool_state.load();
let state = snap.state()?;
if self.polyjuice_creator_id.is_none() {
let polyjuice_creator_id = get_polyjuice_creator_id(
self.generator.rollup_context(),
self.generator.backend_manage(),
&state,
)?;
self.polyjuice_creator_id = polyjuice_creator_id;
}
Ok(self.polyjuice_creator_id)
}

pub(crate) fn package_mem_block(
mem_block: &MemBlock,
output_param: &OutputParam,
Expand Down
21 changes: 4 additions & 17 deletions crates/rpc-server/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use gw_config::{
};
use gw_dynamic_config::manager::{DynamicConfigManager, DynamicConfigReloadResponse};
use gw_generator::generator::CyclesPool;
use gw_generator::utils::{get_polyjuice_creator_id, get_tx_type};
use gw_generator::ArcSwapOption;
use gw_generator::utils::get_tx_type;
use gw_generator::{
error::TransactionError, sudt::build_l2_sudt_script,
verification::transaction::TransactionVerifier, ArcSwap, Generator,
Expand Down Expand Up @@ -149,7 +148,7 @@ pub struct ExecutionTransactionContext {
mem_pool_state: Arc<MemPoolState>,
polyjuice_sender_recover: Arc<PolyjuiceSenderRecover>,
mem_pool_config: MemPoolConfig,
polyjuice_creator_id: ArcSwapOption<u32>,
tx_verifier: Arc<TransactionVerifier>,
}

pub struct SubmitTransactionContext {
Expand Down Expand Up @@ -298,7 +297,7 @@ impl Registry {
mem_pool_state: self.mem_pool_state.clone(),
polyjuice_sender_recover: self.polyjuice_sender_recover.clone(),
mem_pool_config: self.mem_pool_config.clone(),
polyjuice_creator_id: ArcSwapOption::from(None),
tx_verifier: Arc::new(TransactionVerifier::new(self.generator.clone())),
}))
.with_data(Data::new(SubmitTransactionContext {
in_queue_request_map: self.in_queue_request_map.clone(),
Expand Down Expand Up @@ -1040,19 +1039,7 @@ async fn execute_l2transaction(
}

// tx basic verification
if ctx.polyjuice_creator_id.load_full().is_none() {
let polyjuice_creator_id = get_polyjuice_creator_id(
ctx.generator.rollup_context(),
ctx.generator.backend_manage(),
&state,
)?
.map(Arc::new);
ctx.polyjuice_creator_id.store(polyjuice_creator_id);
}

let polyjuice_creator_id = ctx.polyjuice_creator_id.load_full().map(|id| *id);
TransactionVerifier::new(&state, ctx.generator.rollup_context(), polyjuice_creator_id)
.verify(&tx)?;
ctx.tx_verifier.verify(&state, &tx)?;
// verify tx signature
ctx.generator.check_transaction_signature(&state, &tx)?;
// execute tx
Expand Down
1 change: 0 additions & 1 deletion crates/tests/src/testing_tool/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ pub async fn setup_chain_with_account_lock_manage(
node_mode: gw_config::NodeMode::FullNode,
dynamic_config_manager: Default::default(),
has_p2p_sync: false,
polyjuice_creator_id: None,
};
let mem_pool = MemPool::create(args).await.unwrap();
Chain::create(
Expand Down

0 comments on commit cd170ee

Please sign in to comment.