From b355d7aeff7cf705ae44a81f67022c36a2e416c2 Mon Sep 17 00:00:00 2001 From: Eason Gao Date: Wed, 12 Aug 2020 15:39:44 +0800 Subject: [PATCH] refactor(service): change service interface return type (#392) * feat(protocol): add an impl interface macro refactor rename cargo fmt * rebase master * remove useless code --- benchmark/governance/mod.rs | 15 +++-- built-in-services/asset/src/lib.rs | 64 ++++++-------------- built-in-services/metadata/src/lib.rs | 25 +------- built-in-services/multi-signature/src/lib.rs | 34 +++-------- protocol/src/traits/binding.rs | 10 +++ 5 files changed, 46 insertions(+), 102 deletions(-) diff --git a/benchmark/governance/mod.rs b/benchmark/governance/mod.rs index 08d94b119..315c78316 100644 --- a/benchmark/governance/mod.rs +++ b/benchmark/governance/mod.rs @@ -9,6 +9,7 @@ use derive_more::{Display, From}; use binding_macro::{genesis, hook_after, service, tx_hook_after, tx_hook_before}; use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK, StoreMap}; +use protocol::try_service_response; use protocol::types::{Address, Hash, ServiceContext, ServiceContextParams}; use asset::types::TransferPayload; @@ -86,10 +87,9 @@ impl GovernanceService { }; // Pledge the tx failure fee before executed the transaction. - match self.asset.transfer_(&ctx, payload) { - Ok(_) => ServiceResponse::from_succeed("".to_owned()), - Err(e) => ServiceResponse::from_error(e.code, e.error_message), - } + let res = self.asset.transfer_(&ctx, payload); + try_service_response!(res); + ServiceResponse::from_succeed(String::new()) } #[tx_hook_after] @@ -108,10 +108,9 @@ impl GovernanceService { value: 1, }; - match self.asset.transfer_(&ctx, payload) { - Ok(_) => ServiceResponse::from_succeed("".to_owned()), - Err(e) => ServiceResponse::from_error(e.code, e.error_message), - } + let res = self.asset.transfer_(&ctx, payload); + try_service_response!(res); + ServiceResponse::from_succeed(String::new()) } #[hook_after] diff --git a/built-in-services/asset/src/lib.rs b/built-in-services/asset/src/lib.rs index 8ea390517..0f5cd37d7 100644 --- a/built-in-services/asset/src/lib.rs +++ b/built-in-services/asset/src/lib.rs @@ -8,6 +8,7 @@ use std::collections::BTreeMap; use binding_macro::{cycles, genesis, service}; use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK, StoreMap}; +use protocol::try_service_response; use protocol::types::{Address, Bytes, Hash, ServiceContext}; use crate::types::{ @@ -15,57 +16,32 @@ use crate::types::{ GetAllowanceResponse, GetAssetPayload, GetBalancePayload, GetBalanceResponse, InitGenesisPayload, TransferEvent, TransferFromEvent, TransferFromPayload, TransferPayload, }; -macro_rules! impl_assets { - ($self: expr, $method: ident, $ctx: expr) => {{ - let res = $self.$method($ctx.clone()); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; - ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{ - let res = $self.$method($ctx.clone(), $payload); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; -} pub const ASSET_SERVICE_NAME: &str = "asset"; pub trait Assets { - fn create_( - &mut self, - ctx: &ServiceContext, - payload: CreateAssetPayload, - ) -> Result>; + fn create_(&mut self, ctx: &ServiceContext, payload: CreateAssetPayload) + -> ServiceResponse<()>; fn balance_( &self, ctx: &ServiceContext, payload: GetBalancePayload, - ) -> Result>; + ) -> ServiceResponse; - fn transfer_( - &mut self, - ctx: &ServiceContext, - payload: TransferPayload, - ) -> Result<(), ServiceResponse<()>>; + fn transfer_(&mut self, ctx: &ServiceContext, payload: TransferPayload) -> ServiceResponse<()>; fn transfer_from_( &mut self, ctx: &ServiceContext, payload: TransferFromPayload, - ) -> Result<(), ServiceResponse<()>>; + ) -> ServiceResponse<()>; fn allowance_( &self, ctx: &ServiceContext, payload: GetAllowancePayload, - ) -> Result>; + ) -> ServiceResponse; } pub struct AssetService { @@ -78,40 +54,38 @@ impl Assets for AssetService { &mut self, ctx: &ServiceContext, payload: CreateAssetPayload, - ) -> Result> { - impl_assets!(self, create_asset, ctx, payload) + ) -> ServiceResponse<()> { + let res = self.create_asset(ctx.clone(), payload); + try_service_response!(res); + ServiceResponse::from_succeed(()) } fn balance_( &self, ctx: &ServiceContext, payload: GetBalancePayload, - ) -> Result> { - impl_assets!(self, get_balance, ctx, payload) + ) -> ServiceResponse { + self.get_balance(ctx.clone(), payload) } - fn transfer_( - &mut self, - ctx: &ServiceContext, - payload: TransferPayload, - ) -> Result<(), ServiceResponse<()>> { - impl_assets!(self, transfer, ctx, payload) + fn transfer_(&mut self, ctx: &ServiceContext, payload: TransferPayload) -> ServiceResponse<()> { + self.transfer(ctx.clone(), payload) } fn transfer_from_( &mut self, ctx: &ServiceContext, payload: TransferFromPayload, - ) -> Result<(), ServiceResponse<()>> { - impl_assets!(self, transfer_from, ctx, payload) + ) -> ServiceResponse<()> { + self.transfer_from(ctx.clone(), payload) } fn allowance_( &self, ctx: &ServiceContext, payload: GetAllowancePayload, - ) -> Result> { - impl_assets!(self, get_allowance, ctx, payload) + ) -> ServiceResponse { + self.get_allowance(ctx.clone(), payload) } } diff --git a/built-in-services/metadata/src/lib.rs b/built-in-services/metadata/src/lib.rs index be1e84019..ce290ec15 100644 --- a/built-in-services/metadata/src/lib.rs +++ b/built-in-services/metadata/src/lib.rs @@ -5,29 +5,10 @@ use binding_macro::{cycles, genesis, service}; use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK}; use protocol::types::{Metadata, ServiceContext, METADATA_KEY}; -macro_rules! impl_meatdata { - ($self: expr, $method: ident, $ctx: expr) => {{ - let res = $self.$method($ctx.clone()); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; - ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{ - let res = $self.$method($ctx.clone(), $payload); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; -} - pub const METADATA_SERVICE_NAME: &str = "metadata"; pub trait MetaData { - fn get_(&self, ctx: &ServiceContext) -> Result>; + fn get_(&self, ctx: &ServiceContext) -> ServiceResponse; } pub struct MetadataService { @@ -35,8 +16,8 @@ pub struct MetadataService { } impl MetaData for MetadataService { - fn get_(&self, ctx: &ServiceContext) -> Result> { - impl_meatdata!(self, get_metadata, ctx) + fn get_(&self, ctx: &ServiceContext) -> ServiceResponse { + self.get_metadata(ctx.clone()) } } diff --git a/built-in-services/multi-signature/src/lib.rs b/built-in-services/multi-signature/src/lib.rs index 9250aed29..67304b709 100644 --- a/built-in-services/multi-signature/src/lib.rs +++ b/built-in-services/multi-signature/src/lib.rs @@ -8,7 +8,6 @@ use std::collections::HashMap; use std::panic::catch_unwind; use binding_macro::{cycles, genesis, service}; - use common_crypto::{Crypto, Secp256k1}; use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK}; use protocol::types::{Address, Bytes, Hash, ServiceContext, SignedTransaction}; @@ -21,41 +20,22 @@ use crate::types::{ UpdateAccountPayload, VerifySignaturePayload, Witness, }; +pub const MULTI_SIG_SERVICE_NAME: &str = "multi_signature"; const MAX_MULTI_SIGNATURE_RECURSION_DEPTH: u8 = 8; const MAX_PERMISSION_ACCOUNTS: u8 = 16; -macro_rules! impl_multisig { - ($self: expr, $method: ident, $ctx: expr) => {{ - let res = $self.$method($ctx.clone()); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; - ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{ - let res = $self.$method($ctx.clone(), $payload); - if res.is_error() { - Err(ServiceResponse::from_error(res.code, res.error_message)) - } else { - Ok(res.succeed_data) - } - }}; -} -pub const MULTI_SIG_SERVICE_NAME: &str = "multi_signature"; - pub trait MultiSignature { fn verify_signature_( &self, ctx: &ServiceContext, payload: SignedTransaction, - ) -> Result<(), ServiceResponse<()>>; + ) -> ServiceResponse<()>; fn generate_account_( &mut self, ctx: &ServiceContext, payload: GenerateMultiSigAccountPayload, - ) -> Result>; + ) -> ServiceResponse; } pub struct MultiSignatureService { @@ -67,16 +47,16 @@ impl MultiSignature for MultiSignatureService { &self, ctx: &ServiceContext, payload: SignedTransaction, - ) -> Result<(), ServiceResponse<()>> { - impl_multisig!(self, verify_signature, ctx, payload) + ) -> ServiceResponse<()> { + self.verify_signature(ctx.clone(), payload) } fn generate_account_( &mut self, ctx: &ServiceContext, payload: GenerateMultiSigAccountPayload, - ) -> Result> { - impl_multisig!(self, generate_account, ctx, payload) + ) -> ServiceResponse { + self.generate_account(ctx.clone(), payload) } } diff --git a/protocol/src/traits/binding.rs b/protocol/src/traits/binding.rs index a0f6c7bd5..18d9fda19 100644 --- a/protocol/src/traits/binding.rs +++ b/protocol/src/traits/binding.rs @@ -5,6 +5,16 @@ use crate::traits::{ExecutorParams, ServiceResponse}; use crate::types::{Address, Block, Hash, MerkleRoot, Receipt, ServiceContext, SignedTransaction}; use crate::ProtocolResult; +#[macro_export] +macro_rules! try_service_response { + ($service_resp: expr) => {{ + if $service_resp.is_error() { + return ServiceResponse::from_error($service_resp.code, $service_resp.error_message); + } + $service_resp.succeed_data + }}; +} + pub trait SDKFactory { fn get_sdk(&self, name: &str) -> ProtocolResult; }