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

Support external eth_sign #5481

Merged
merged 6 commits into from Apr 27, 2017
Merged

Support external eth_sign #5481

merged 6 commits into from Apr 27, 2017

Conversation

tomusdrw
Copy link
Collaborator

Related #5469
Decrypt support is still to be done.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. M7-ui labels Apr 20, 2017
@tomusdrw tomusdrw requested review from jacogr and debris April 20, 2017 15:13
@@ -85,7 +85,7 @@ export default class RequestOrigin extends Component {
<span>
<FormattedMessage
id='signer.requestOrigin.rpc'
defaultMessage='via RPC {rpc}'
defaultMessage='via RPC {url}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Erk, nice catch.

@jacogr
Copy link
Contributor

jacogr commented Apr 21, 2017

JS side LGTM.

/// Parse bytes as a signature encoded as VRS (V in "Electrum" notation).
/// May return empty (invalid) signature if given data has invalid length.
pub fn from_vrs(data: &[u8]) -> Self {
if data.len() != 65 || data[0] < 27 {
Copy link
Contributor

Choose a reason for hiding this comment

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

probably want to ensure data[0] < 29, too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not really trying to validate the signature yet (there is a separate function for this, see Signature#is_valid), the check is here to prevent panics/overflows.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 25, 2017
@gavofyork gavofyork merged commit 28dcbc6 into master Apr 27, 2017
@gavofyork gavofyork deleted the ui-eth-sign branch April 27, 2017 16:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants