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

rpc: implement personal_importRawKey #552

Merged
merged 17 commits into from
Sep 30, 2020
Merged

rpc: implement personal_importRawKey #552

merged 17 commits into from
Sep 30, 2020

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Sep 29, 2020 •
edited
Loading

closes #368
closes #414

@fedekunze fedekunze requested a review from noot as a code owner September 29, 2020 13:49
@codecov
Copy link

codecov bot commented Sep 29, 2020 •
edited
Loading

Codecov Report

Merging #552 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development     #552   +/-   ##
============================================
  Coverage        70.71%   70.71%           
============================================
  Files               42       42           
  Lines             2339     2339           
============================================
  Hits              1654     1654           
  Misses             541      541           
  Partials           144      144
Impacted Files Coverage Δ
crypto/algorithm.go 76.47% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 592eca9...0297bfc. Read the comment docs.

noot
noot reviewed Sep 29, 2020
View reviewed changes
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

looks good! could you add a RPC test for this that imports the raw key and unlocks it?

@fedekunze
Copy link
Contributor Author

added the test, lmk if it's good @noot

@fedekunze fedekunze added rpc Ethereum JSON-RPC related issues and PRs Status: Needs Review labels Sep 29, 2020
fedekunze
fedekunze commented Sep 29, 2020
View reviewed changes
rpc/personal_api.go Outdated Show resolved Hide resolved
fedekunze
fedekunze commented Sep 29, 2020
View reviewed changes
rpc/personal_api.go Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Sep 29, 2020 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ fedekunze
✅ noot
❌ marbar3778
You have signed the CLA already but the status is still pending? Let us recheck it.

noot
noot reviewed Sep 29, 2020
View reviewed changes
rpc/personal_api.go Outdated
@@ -128,6 +146,8 @@ func (e *PersonalEthAPI) LockAccount(address common.Address) bool {
return true
}

e.logger.Debug("account unlocked", "address", address.String())
Copy link
Contributor

Choose a reason for hiding this comment

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

this log should be before the return true

noot
noot approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

works for me! just a couple minor comments

@noot
Copy link
Contributor

noot commented Sep 29, 2020 •
edited
Loading

I think ethAPI.cliCtx and personalAPI.cliCtx need to be consolidated since right now I'm unable to send a transaction from a new or imported account due to a nonexistent account error. this seems to be happening because the personalAPI's cliCtx.Keybase is getting updated but not the ethAPI's cliCtx.Keybase.

could do this in a follow up

noot
noot approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

still getting the nonexistent account error, not sure why since it's using the same keybase now :/ we can open an issue for it

@noot noot merged commit 811ca7f into development Sep 30, 2020
@noot noot deleted the personal_importRawKey branch September 30, 2020 00:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@noot noot noot approved these changes

@araskachoi araskachoi Awaiting requested review from araskachoi

@J-Thompson12 J-Thompson12 Awaiting requested review from J-Thompson12

Assignees
No one assigned
Labels
rpc Ethereum JSON-RPC related issues and PRs Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

implement web3 personal API Web3 personal API
4 participants
@fedekunze @CLAassistant @noot @tac0turtle