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.

Sign up for GitHub

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 runtime API ConvertTransactionRuntime API #559

Merged
merged 3 commits into from
Feb 2, 2022
Merged

Add runtime API ConvertTransactionRuntime API #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2092f89
Add runtime API `ConvertTransactionRuntime API`
librelois Jan 25, 2022
6e3e78a
ref: make transaction_converter optional
librelois Jan 27, 2022
712ed44
rustfmt
librelois Jan 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ members = [
"primitives/self-contained",
"template/node",
"template/runtime",
]
]
147 changes: 126 additions & 21 deletions client/rpc/src/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use fc_rpc_core::{
},
EthApi as EthApiT, EthFilterApi as EthFilterApiT, NetApi as NetApiT, Web3Api as Web3ApiT,
};
use fp_rpc::{ConvertTransaction, EthereumRuntimeRPCApi, TransactionStatus};
use fp_rpc::{ConvertTransactionRuntimeApi, EthereumRuntimeRPCApi, TransactionStatus};
use futures::{future::TryFutureExt, StreamExt};
use jsonrpc_core::{futures::future, BoxFuture, Result};
use lru::LruCache;
Expand Down Expand Up @@ -68,7 +68,7 @@ pub struct EthApi<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> {
pool: Arc<P>,
graph: Arc<Pool<A>>,
client: Arc<C>,
convert_transaction: CT,
convert_transaction: Option<CT>,
network: Arc<NetworkService<B, H>>,
is_authority: bool,
signers: Vec<Box<dyn EthSigner>>,
Expand All @@ -84,8 +84,10 @@ pub struct EthApi<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> {
impl<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> EthApi<B, C, P, CT, BE, H, A>
where
C: ProvideRuntimeApi<B>,
C::Api: EthereumRuntimeRPCApi<B>,
C::Api: BlockBuilder<B>,
C::Api: sp_api::ApiExt<B>
+ BlockBuilder<B>
+ ConvertTransactionRuntimeApi<B>
+ EthereumRuntimeRPCApi<B>,
B: BlockT<Hash = H256> + Send + Sync + 'static,
A: ChainApi<Block = B> + 'static,
C: Send + Sync + 'static,
Expand All @@ -94,7 +96,7 @@ where
client: Arc<C>,
pool: Arc<P>,
graph: Arc<Pool<A>>,
convert_transaction: CT,
convert_transaction: Option<CT>,
network: Arc<NetworkService<B, H>>,
signers: Vec<Box<dyn EthSigner>>,
overrides: Arc<OverrideHandle<B>>,
Expand Down Expand Up @@ -535,15 +537,17 @@ impl<B, C, P, CT, BE, H: ExHashT, A> EthApiT for EthApi<B, C, P, CT, BE, H, A>
where
C: ProvideRuntimeApi<B> + StorageProvider<B, BE>,
C: HeaderBackend<B> + HeaderMetadata<B, Error = BlockChainError> + 'static,
C::Api: EthereumRuntimeRPCApi<B>,
C::Api: BlockBuilder<B>,
C::Api: sp_api::ApiExt<B>
+ BlockBuilder<B>
+ ConvertTransactionRuntimeApi<B>
+ EthereumRuntimeRPCApi<B>,
BE: Backend<B> + 'static,
BE::State: StateBackend<BlakeTwo256>,
B: BlockT<Hash = H256> + Send + Sync + 'static,
C: Send + Sync + 'static,
P: TransactionPool<Block = B> + Send + Sync + 'static,
A: ChainApi<Block = B> + 'static,
CT: ConvertTransaction<<B as BlockT>::Extrinsic> + Send + Sync + 'static,
CT: fp_rpc::ConvertTransaction<<B as BlockT>::Extrinsic> + Send + Sync + 'static,
{
fn protocol_version(&self) -> Result<u64> {
Ok(1)
Expand Down Expand Up @@ -1023,14 +1027,65 @@ where
None => return Box::pin(future::err(internal_err("no signer available"))),
};
let transaction_hash = transaction.hash();

let block_hash = BlockId::hash(self.client.info().best_hash);
let api_version = match self
.client
.runtime_api()
.api_version::<dyn ConvertTransactionRuntimeApi<B>>(&block_hash)
{
Ok(api_version) => api_version,
_ => return Box::pin(future::err(internal_err("cannot access runtime api"))),
};

let extrinsic = match api_version {
Some(2) => match self
.client
.runtime_api()
.convert_transaction(&block_hash, transaction)
{
Ok(extrinsic) => extrinsic,
Err(_) => return Box::pin(future::err(internal_err("cannot access runtime api"))),
},
Some(1) => {
if let ethereum::TransactionV2::Legacy(legacy_transaction) = transaction {
// To be compatible with runtimes that do not support transactions v2
#[allow(deprecated)]
match self
.client
.runtime_api()
.convert_transaction_before_version_2(&block_hash, legacy_transaction)
{
Ok(extrinsic) => extrinsic,
Err(_) => {
return Box::pin(future::err(internal_err("cannot access runtime api")))
}
}
} else {
return Box::pin(future::err(internal_err(
"This runtime not support eth transactions v2",
)));
}
}
None => {
if let Some(ref convert_transaction) = self.convert_transaction {
convert_transaction.convert_transaction(transaction.clone())
} else {
return Box::pin(future::err(internal_err(
"No TransactionConverter is provided and the runtime api ConvertTransactionRuntimeApi is not found"
)));
}
}
_ => {
return Box::pin(future::err(internal_err(
"ConvertTransactionRuntimeApi version not supported",
)))
}
};

Box::pin(
self.pool
.submit_one(
&BlockId::hash(hash),
TransactionSource::Local,
self.convert_transaction
.convert_transaction(transaction.clone()),
)
.submit_one(&block_hash, TransactionSource::Local, extrinsic)
.map_ok(move |_| transaction_hash)
.map_err(|err| {
internal_err(format!("submit transaction to pool failed: {:?}", err))
Expand Down Expand Up @@ -1064,15 +1119,65 @@ where
};

let transaction_hash = transaction.hash();
let hash = self.client.info().best_hash;

let block_hash = BlockId::hash(self.client.info().best_hash);
let api_version = match self
.client
.runtime_api()
.api_version::<dyn ConvertTransactionRuntimeApi<B>>(&block_hash)
{
Ok(api_version) => api_version,
_ => return Box::pin(future::err(internal_err("cannot access runtime api"))),
};

let extrinsic = match api_version {
Some(2) => match self
.client
.runtime_api()
.convert_transaction(&block_hash, transaction)
{
Ok(extrinsic) => extrinsic,
Err(_) => return Box::pin(future::err(internal_err("cannot access runtime api"))),
},
Some(1) => {
if let ethereum::TransactionV2::Legacy(legacy_transaction) = transaction {
// To be compatible with runtimes that do not support transactions v2
#[allow(deprecated)]
match self
.client
.runtime_api()
.convert_transaction_before_version_2(&block_hash, legacy_transaction)
{
Ok(extrinsic) => extrinsic,
Err(_) => {
return Box::pin(future::err(internal_err("cannot access runtime api")))
}
}
} else {
return Box::pin(future::err(internal_err(
"This runtime not support eth transactions v2",
)));
}
}
None => {
if let Some(ref convert_transaction) = self.convert_transaction {
convert_transaction.convert_transaction(transaction.clone())
} else {
return Box::pin(future::err(internal_err(
"No TransactionConverter is provided and the runtime api ConvertTransactionRuntimeApi is not found"
)));
}
}
_ => {
return Box::pin(future::err(internal_err(
"ConvertTransactionRuntimeApi version not supported",
)))
}
};

Box::pin(
self.pool
.submit_one(
&BlockId::hash(hash),
TransactionSource::Local,
self.convert_transaction
.convert_transaction(transaction.clone()),
)
.submit_one(&block_hash, TransactionSource::Local, extrinsic)
.map_ok(move |_| transaction_hash)
.map_err(|err| {
internal_err(format!("submit transaction to pool failed: {:?}", err))
Expand Down
18 changes: 18 additions & 0 deletions primitives/rpc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,26 @@ sp_api::decl_runtime_apis! {
/// Return the elasticity multiplier.
fn elasticity() -> Option<Permill>;
}

#[api_version(2)]
pub trait ConvertTransactionRuntimeApi {
fn convert_transaction(transaction: ethereum::TransactionV2) -> <Block as BlockT>::Extrinsic;
Copy link
Member

@sorpaas sorpaas Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Consider changing this to with RLP Vec<u8> as input, and return Option<Extrinsic> where None indicates that the conversion failed.

Ethereum transaction's RLP encoding is backward and future compatible, but not really the SCALE encoding, as that's internal. Doing the above ensures that when runtime updates to support a new transaction type, you can immediately use it without waiting for old nodes to update. With this you can also remove the versioning.

Also, is there a particular reason that this isn't put into EthereumRuntimeRPCApi?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm I realized that our EthereumRuntimeRPCApi is not forward compatible anyway, so the above (using RLP Vec<u8> as input) may not provide much benefits.

The other points still stands -- maybe consider moving this to EthereumRuntimeRPCApi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided to have it in a separate trait so that the client can ask if the API is present or not.
Indeed, substrate does not allow to ask if a particular method of a runtime api is present or not.

#[changed_in(2)]
fn convert_transaction(transaction: ethereum::TransactionV0) -> <Block as BlockT>::Extrinsic;
}
}

pub trait ConvertTransaction<E> {
fn convert_transaction(&self, transaction: ethereum::TransactionV2) -> E;
}

// `NoTransactionConverter` is a non-instantiable type (an enum with no variants),
// so we are guaranteed at compile time that `NoTransactionConverter` can never be instantiated.
pub enum NoTransactionConverter {}
Copy link
Member

@sorpaas sorpaas Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is this actually used? Do we actually have a situation where transaction conversion is just not possible or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have this type, then why do you still have an Option<_> in Eth::new?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I get it. I use this only as a type parameter. and use None as the value. That's why it's not instantiable.

impl<E> ConvertTransaction<E> for NoTransactionConverter {
// `convert_transaction` is a method taking `&self` as a parameter, so it can only be called via an instance of type Self,
// so we are guaranteed at compile time that this method can never be called.
fn convert_transaction(&self, _transaction: ethereum::TransactionV2) -> E {
unreachable!()
}
}
7 changes: 5 additions & 2 deletions template/node/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ where
C: ProvideRuntimeApi<Block> + StorageProvider<Block, BE> + AuxStore,
C: HeaderBackend<Block> + HeaderMetadata<Block, Error = BlockChainError>,
C: Send + Sync + 'static,
C::Api: fp_rpc::EthereumRuntimeRPCApi<Block>,
C::Api: sp_api::ApiExt<Block>
+ fp_rpc::EthereumRuntimeRPCApi<Block>
+ fp_rpc::ConvertTransactionRuntimeApi<Block>,
BE: Backend<Block> + 'static,
BE::State: StateBackend<BlakeTwo256>,
{
Expand Down Expand Up @@ -120,6 +122,7 @@ where
C::Api: substrate_frame_rpc_system::AccountNonceApi<Block, AccountId, Index>,
C::Api: BlockBuilder<Block>,
C::Api: pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi<Block, Balance>,
C::Api: fp_rpc::ConvertTransactionRuntimeApi<Block>,
C::Api: fp_rpc::EthereumRuntimeRPCApi<Block>,
P: TransactionPool<Block = Block> + 'static,
A: ChainApi<Block = Block> + 'static,
Expand Down Expand Up @@ -169,7 +172,7 @@ where
client.clone(),
pool.clone(),
graph,
frontier_template_runtime::TransactionConverter,
Some(frontier_template_runtime::TransactionConverter),
network.clone(),
signers,
overrides.clone(),
Expand Down
8 changes: 8 additions & 0 deletions template/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,14 @@ impl_runtime_apis! {
}
}

impl fp_rpc::ConvertTransactionRuntimeApi<Block> for Runtime {
fn convert_transaction(transaction: EthereumTransaction) -> <Block as BlockT>::Extrinsic {
UncheckedExtrinsic::new_unsigned(
pallet_ethereum::Call::<Runtime>::transact { transaction }.into(),
)
}
}

impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi<
Block,
Balance,
Expand Down