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

Signer provenance #4477

Merged
merged 9 commits into from
Feb 14, 2017
Merged

Signer provenance #4477

merged 9 commits into from
Feb 14, 2017

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Feb 8, 2017

Exposes signer provenance in RPC and displays it in the UI.

Currently IPC doesn't support metadata extraction, so we cannot properly identify IPC requests yet.
It should be added when paritytech/jsonrpc#17 is implemented.

GUI:
screenshot from 2017-02-08 13-42-27

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. M7-ui labels Feb 8, 2017
@@ -305,7 +305,7 @@ impl<T> TraceDatabase for TraceDB<T> where T: DatabaseExtras {
}

fn trace(&self, block_number: BlockNumber, tx_position: usize, trace_position: Vec<usize>) -> Option<LocalizedTrace> {
let trace_position_deq = trace_position.into_iter().collect();
let trace_position_deq = trace_position.into_iter().collect::<VecDeque<usize>>();
Copy link
Contributor

@rphmeier rphmeier Feb 8, 2017

Choose a reason for hiding this comment

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

can be more efficiently done with a simple trace_position.into()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 Good point. Had to touch this line since it wasn't compiling on latest nightly.

};

static propTypes = {
origin: PropTypes.oneOfType([
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 having two props, e.g. type and origin here, one containing rpc/ipc/etc and one containing the data necessary to display it. Better to read and cleaner than passing an object as a prop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

K, will pre-process RPC response.

expect(shallow(
<RequestOrigin origin={ { signer: '0x12345' } } />,
context
).text()).to.equal('Requested by UI session<Connect(IdentityIcon) />');
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO "Requested by from the UI" would be more intuitive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so by or from?

Copy link
Contributor

@derhuerst derhuerst Feb 9, 2017

Choose a reason for hiding this comment

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

Edit: github removes stuff from my comment, sry for the confusion.

both. requested by <identity icon> from the UI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But <identity icon> doesn't mean an account, but UI session.

@gavofyork gavofyork added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 9, 2017
@gavofyork
Copy link
Contributor

marked inprogress as build is failing.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Feb 11, 2017
@rphmeier
Copy link
Contributor

Core portion LGTM

@arkpar arkpar added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 13, 2017
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Feb 14, 2017
@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 14, 2017
@gavofyork gavofyork merged commit 5369a12 into master Feb 14, 2017
@gavofyork gavofyork deleted the signer-provenance branch February 14, 2017 21:45
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.

6 participants