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

Exposing engine extra info in block RPC #3169

Merged
merged 3 commits into from
Nov 4, 2016
Merged

Exposing engine extra info in block RPC #3169

merged 3 commits into from
Nov 4, 2016

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Nov 4, 2016

Addresses: #3148

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 4, 2016
@@ -1195,6 +1195,10 @@ impl MiningBlockChainClient for Client {
self.engine.schedule(&self.latest_env_info())
}

fn engine(&self) -> &Engine {
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd prefer not to expose a complex type like Engine over the BlockChainClient trait; it opens up too much functionality over the interface and destroys the possibility of IPC.

that's why i explicitly stated a proxy function for extra_info.

uncles: vec![],
transactions: BlockTransactions::Hashes(vec![].into()),
size: Some(69.into()),
};

let serialized = serde_json::to_string(&block).unwrap();
assert_eq!(serialized, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","author":"0x0000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x0","gasUsed":"0x0","gasLimit":"0x0","extraData":"0x","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","timestamp":"0x0","difficulty":"0x0","totalDifficulty":"0x0","sealFields":["0x","0x"],"uncles":[],"transactions":[],"size":"0x45"}"#);
assert_eq!(serialized, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","author":"0x0000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x0","gasUsed":"0x0","gasLimit":"0x0","extraData":"0x","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","timestamp":"0x0","difficulty":"0x0","totalDifficulty":"0x0","sealFields":["0x","0x"],"extraInfo":{"mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000"},"uncles":[],"transactions":[],"size":"0x45"}"#);
Copy link
Contributor

@gavofyork gavofyork Nov 4, 2016

Choose a reason for hiding this comment

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

as i pointed out in the issue, the point of extra_info is that the keys are placed directly into the block/header JSON object. this is required in order to be compatible with the existing geth behaviour.

i went to quite some effort in the original issue describing why this was going to be such a pain.

@gavofyork gavofyork added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 4, 2016
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Nov 4, 2016
@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Nov 4, 2016

Fixed. Sorry for the issues, I've misunderstood the solution you proposed.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. B0-patch and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 4, 2016
@gavofyork
Copy link
Contributor

all good.

@gavofyork gavofyork merged commit f31d42d into master Nov 4, 2016
@gavofyork gavofyork deleted the extra-info branch November 4, 2016 16:35
arkpar pushed a commit that referenced this pull request Nov 4, 2016
* Exposing extra info in RPC

* Proper serialization and client trait API
arkpar added a commit that referenced this pull request Nov 4, 2016
* sendRawTransaction invalid RLP error

* Returning proper error for estimate_gas

* Exposing engine extra info in block RPC (#3169)

* Exposing extra info in RPC

* Proper serialization and client trait API

* Fixing possible race condition in ethcore_hashContent (#3191)

* Remove dapp logos (GHH points to dapp-assets) (#3192)
@stevenroose
Copy link

Great! When will this land in the beta-release Docker image? :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants