Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[light/jsonrpc] Provide the actual account for eth_coinbase RPC and unify error handeling for light and full client #9383

Merged
merged 6 commits into from
Sep 5, 2018
Merged

[light/jsonrpc] Provide the actual account for eth_coinbase RPC and unify error handeling for light and full client #9383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7f1c4f8
Provide the actual `account` for eth_coinbase
niklasad1 Aug 20, 2018
4fafecc
full client `eth_coinbase` return err
niklasad1 Aug 28, 2018
8b53092
Remove needless blocks on single import
niklasad1 Aug 30, 2018
e5e6faf
Remove needless `static` lifetime on const
niklasad1 Aug 30, 2018
bc05784
Fix `rpc_eth_author` test
niklasad1 Sep 1, 2018
ff158e2
Merge remote-tracking branch 'upstream/master' into light/provide-acc…
niklasad1 Sep 5, 2018
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
16 changes: 10 additions & 6 deletions rpc/src/v1/impls/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use ethcore::log_entry::LogEntry;
use ethcore::miner::{self, MinerService};
use ethcore::snapshot::SnapshotService;
use ethcore::encoded;
use sync::{SyncProvider};
use sync::SyncProvider;
use miner::external::ExternalMinerService;
use transaction::{SignedTransaction, LocalizedTransaction};

Expand All @@ -52,7 +52,7 @@ use v1::types::{
};
use v1::metadata::Metadata;

const EXTRA_INFO_PROOF: &'static str = "Object exists in blockchain (fetched earlier), extra_info is always available if object exists; qed";
const EXTRA_INFO_PROOF: &str = "Object exists in blockchain (fetched earlier), extra_info is always available if object exists; qed";

/// Eth RPC options
pub struct EthClientOptions {
Expand Down Expand Up @@ -500,12 +500,16 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
}

fn author(&self) -> Result<RpcH160> {
let mut miner = self.miner.authoring_params().author;
let miner = self.miner.authoring_params().author;
if miner == 0.into() {
miner = self.accounts.accounts().ok().and_then(|a| a.get(0).cloned()).unwrap_or_default();
self.accounts.accounts()
.ok()
.and_then(|a| a.first().cloned())
.map(From::from)
.ok_or_else(|| errors::account("No accounts were found", ""))
} else {
Ok(RpcH160::from(miner))
}

Ok(RpcH160::from(miner))
}

fn is_mining(&self) -> Result<bool> {
Expand Down
6 changes: 5 additions & 1 deletion rpc/src/v1/impls/light/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ impl<T: LightChainClient + 'static> Eth for EthClient<T> {
}

fn author(&self) -> Result<RpcH160> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for my education: what is the reason for rpc-specific hashtypes like RpcH160?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Historical reasons - hash types were not JSON-serializable in the past, I think currently we could get rid of the wrappers and just use the regular types

Ok(Default::default())
self.accounts.accounts()
.ok()
.and_then(|a| a.first().cloned())
.map(From::from)
.ok_or_else(|| errors::account("No accounts were found", ""))
}

fn is_mining(&self) -> Result<bool> {
Expand Down
12 changes: 7 additions & 5 deletions rpc/src/v1/tests/mocked/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,25 +359,27 @@ fn rpc_eth_author() {
let make_res = |addr| r#"{"jsonrpc":"2.0","result":""#.to_owned() + &format!("0x{:x}", addr) + r#"","id":1}"#;
let tester = EthTester::default();

let req = r#"{
let request = r#"{
"jsonrpc": "2.0",
"method": "eth_coinbase",
"params": [],
"id": 1
}"#;

// No accounts - returns zero
assert_eq!(tester.io.handle_request_sync(req), Some(make_res(Address::zero())));
let response = r#"{"jsonrpc":"2.0","error":{"code":-32023,"message":"No accounts were found","data":"\"\""},"id":1}"#;

// No accounts - returns an error indicating that no accounts were found
assert_eq!(tester.io.handle_request_sync(request), Some(response.to_string()));

// Account set - return first account
let addr = tester.accounts_provider.new_account(&"123".into()).unwrap();
assert_eq!(tester.io.handle_request_sync(req), Some(make_res(addr)));
assert_eq!(tester.io.handle_request_sync(request), Some(make_res(addr)));

for i in 0..20 {
let addr = tester.accounts_provider.new_account(&format!("{}", i).into()).unwrap();
tester.miner.set_author(addr.clone(), None).unwrap();

assert_eq!(tester.io.handle_request_sync(req), Some(make_res(addr)));
assert_eq!(tester.io.handle_request_sync(request), Some(make_res(addr)));
}
}

Expand Down