Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: remove personal RPC namespace #30704

Merged
merged 4 commits into from
Oct 31, 2024
Merged

all: remove personal RPC namespace #30704

merged 4 commits into from
Oct 31, 2024

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Oct 31, 2024

This PR is a first step towards removing account management from geth, and contains a lot of the user-facing changes.

With this PR, the personal namespace disappears. Note: personal namespace has been deprecated for quite some time (since #26390 1 year and 8 months ago), and users who have wanted to use it has been forced to used the flag --rpc.enabledeprecatedpersonal. So I think it's fairly non-controversial to drop it at this point.

Specifically, this means:

  • Account/wallet listing
    -personal.getListAccounts
    -personal.listAccounts
    -personal.getListWallets
    -personal.listWallets
  • Lock/unlock
    -personal.lockAccount
    -personal.openWallet
    -personal.unlockAccount
  • Sign ops
    -personal.sign
    -personal.sendTransaction
    -personal.signTransaction
  • Imports / inits
    -personal.deriveAccount
    -personal.importRawKey
    -personal.initializeWallet
    -personal.newAccount
    -personal.unpair
  • Other:
    -personal.ecRecover

The underlying keystores and account managent code is still in place, which means that geth --dev still works as expected, so that e.g. the example below still works:

> eth.sendTransaction({data:"0x6060", value: 1, from:eth.accounts[0]})

Also, ethkey and clef are untouched.

With the removal of personal, as far as I know we have no more API methods which contain credentials, and if we want to implement logging-capabilities of RPC ingress payload, it would be possible after this.

@fjl fjl self-assigned this Oct 31, 2024
@@ -1400,10 +1395,6 @@ func SetNodeConfig(ctx *cli.Context, cfg *node.Config) {
cfg.JWTSecret = ctx.String(JWTSecretFlag.Name)
}

if ctx.IsSet(EnablePersonal.Name) {
cfg.EnablePersonal = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we exit geth here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or at least print a warning?

Copy link

Choose a reason for hiding this comment

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

Warning sgtm!

@fjl fjl changed the title cmd/geth: remove personal namespace all: remove personal RPC namespace Oct 31, 2024
@fjl fjl merged commit f3b4bbb into ethereum:master Oct 31, 2024
3 checks passed
@fjl fjl added this to the 1.14.12 milestone Oct 31, 2024
GrapeBaBa pushed a commit to optimism-java/shisui that referenced this pull request Nov 2, 2024
This PR is a first step towards removing account management from geth,
and contains a lot of the user-facing changes.

With this PR, the `personal` namespace disappears. **Note**: `personal`
namespace has been deprecated for quite some time (since
ethereum#26390 1 year and 8 months
ago), and users who have wanted to use it has been forced to used the
flag `--rpc.enabledeprecatedpersonal`. So I think it's fairly
non-controversial to drop it at this point.

Specifically, this means:

- Account/wallet listing
  -`personal.getListAccounts`
  -`personal.listAccounts`
  -`personal.getListWallets`
  -`personal.listWallets`
- Lock/unlock
  -`personal.lockAccount`
  -`personal.openWallet`
  -`personal.unlockAccount`
- Sign ops
  -`personal.sign`
  -`personal.sendTransaction`
  -`personal.signTransaction`
- Imports / inits
  -`personal.deriveAccount`
  -`personal.importRawKey`
  -`personal.initializeWallet`
  -`personal.newAccount`
  -`personal.unpair`
- Other:
  -`personal.ecRecover`

The underlying keystores and account managent code is still in place,
which means that `geth --dev` still works as expected, so that e.g. the
example below still works:

```
> eth.sendTransaction({data:"0x6060", value: 1, from:eth.accounts[0]})
```

Also, `ethkey` and `clef` are untouched.

With the removal of `personal`, as far as I know we have no more API
methods which contain credentials, and if we want to implement
logging-capabilities of RPC ingress payload, it would be possible after
this.

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
holiman added a commit that referenced this pull request Nov 19, 2024
This PR is a first step towards removing account management from geth,
and contains a lot of the user-facing changes.

With this PR, the `personal` namespace disappears. **Note**: `personal`
namespace has been deprecated for quite some time (since
#26390 1 year and 8 months
ago), and users who have wanted to use it has been forced to used the
flag `--rpc.enabledeprecatedpersonal`. So I think it's fairly
non-controversial to drop it at this point.

Specifically, this means: 

- Account/wallet listing
  -`personal.getListAccounts`  
  -`personal.listAccounts`     
  -`personal.getListWallets`   
  -`personal.listWallets`      
- Lock/unlock
  -`personal.lockAccount`      
  -`personal.openWallet`       
  -`personal.unlockAccount`
- Sign ops
  -`personal.sign`             
  -`personal.sendTransaction`  
  -`personal.signTransaction`  
- Imports / inits
  -`personal.deriveAccount`    
  -`personal.importRawKey`     
  -`personal.initializeWallet` 
  -`personal.newAccount`       
  -`personal.unpair` 
- Other: 
  -`personal.ecRecover`        


The underlying keystores and account managent code is still in place,
which means that `geth --dev` still works as expected, so that e.g. the
example below still works:

```
> eth.sendTransaction({data:"0x6060", value: 1, from:eth.accounts[0]})
```	

Also, `ethkey` and `clef` are untouched. 

With the removal of `personal`, as far as I know we have no more API
methods which contain credentials, and if we want to implement
logging-capabilities of RPC ingress payload, it would be possible after
this.

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants