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

feat: Implement EIP-1024 #11284

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Implement EIP-1024 #11284

wants to merge 2 commits into from

Conversation

hskang9
Copy link

@hskang9 hskang9 commented Nov 24, 2019

resolves #9893

@parity-cla-bot
Copy link

It looks like @hskang9 signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@jam10o-new jam10o-new added A0-pleasereview 🤓 Pull request needs code review. F8-enhancement 🎊 An additional feature request. labels Nov 24, 2019
@jam10o-new
Copy link
Contributor

jam10o-new commented Nov 24, 2019

the next step here would probably be to expose these methods as an rpc endpoint
for reference #9631 is a PR with similar scope that is a good example of what needs to be done

util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
let send_pk = Public::from_unsafe_slice(&pk_byte32).unwrap();
let cipher_bytes = hex::decode(encrypted_data.ciphertext).unwrap();
let nonce_bytes = hex::decode(encrypted_data.nonce).unwrap();
let nonce = array_ref!(nonce_bytes, 0, 24).clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

use copy_from_slice instead and remove dependency to arrayref

let nonce = array_ref!(nonce_bytes, 0, 24).clone();
println!("cipher_bytes: {:?}\n nonce: {:?}\n send_pk: {:?}\n recv_sk: {:?}\n", cipher_bytes, nonce, hex::encode(*send_pk), hex::encode(recv_sk.secret()) );
let result = saltbabe::crypto_box::open(&cipher_bytes, &nonce, &send_pk, &recv_sk.secret()).unwrap();
return Ok(str::from_utf8(&result.clone()).unwrap().to_string());
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be something like:

    saltbabe::crypto_box::open(&cipher_bytes, &nonce, &send_pk, &recv_sk.secret())
		.and_then(|r| String::from_utf8(r).map_err(|e| Error::Custom(e.to_string())))


pub fn gen_keypair() -> KeyPair<Secret, Public> {

KeyPair::<Secret, Public>::generate_keypair().unwrap()
Copy link
Collaborator

@niklasad1 niklasad1 Nov 25, 2019

Choose a reason for hiding this comment

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

either change so that this function returns Result<KeyPair, Error> or use expect! and provide a proof why is it is infallible.

util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved

#[cfg(test)]
mod tests {
extern crate saltbabe;
Copy link
Collaborator

Choose a reason for hiding this comment

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

extern crate not needed

@@ -0,0 +1,139 @@
extern crate saltbabe;
Copy link
Collaborator

Choose a reason for hiding this comment

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

the file is missing license header.

@niklasad1
Copy link
Collaborator

FYI, this PR depends on rust-crypto which is not maintained anymore...

description = "EIP-1024 encryption"
keywords = ["eip-1024", "eip1024", "eip"]
license = "GPL-3.0"
authors = ["hskang9 <[email protected]>"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should include Parity Technologies

Copy link
Author

Choose a reason for hiding this comment

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

ok

@@ -0,0 +1,674 @@
GNU GENERAL PUBLIC LICENSE
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is already a copy in the main directory.

Copy link
Author

Choose a reason for hiding this comment

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

ok

use saltbabe::traits::FromUnsafeSlice;

fn main() {
let bob_sk = "mJxmrVq8pfeR80HMZBTkjV+RiND1lqPqLuCdDUiduis=";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is correctly indented using tabs, the rest uses spaces. Please reformat to all tabs.

Copy link
Author

Choose a reason for hiding this comment

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

ok

const VERSION: &str = "x25519-xsalsa20-poly1305";

#[derive(Debug, Clone)]
pub struct EncryptedData {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice to enforce documentation for all public types:

#![warn(missing_docs)]

Copy link
Author

Choose a reason for hiding this comment

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

ok

util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
util/EIP-1024/src/lib.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,139 @@
extern crate saltbabe;
#[macro_use]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use tabs across the entire codebase, could you please reformat this file (and all other files) accordingly?

Copy link
Author

Choose a reason for hiding this comment

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

ok

edition = "2018"

[dependencies]
saltbabe = "0.1.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This crate is missing a link to the repo, where can we inspect the code easily and file issues in case of problems?

Copy link
Author

Choose a reason for hiding this comment

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

ok I should add that.

@tomusdrw
Copy link
Collaborator

Exposing over RPC is missing. This crate is now only standalone, I don't think it full covers #9893

Copy link
Author

@hskang9 hskang9 left a comment

Choose a reason for hiding this comment

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

On my way to make RPC in traits and impl folder. I will work on eth.rs file.

@niklasad1
Copy link
Collaborator

@hskang9 status on this?

@niklasad1 niklasad1 added A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 26, 2020
@hskang9
Copy link
Author

hskang9 commented Mar 12, 2020

Sorry for late response. I thought this was excluded in Eth 2.0 timeline, so I thought this was halted. I will try to work on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. F8-enhancement 🎊 An additional feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement EIP 1024 encryption methods
5 participants