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

Forbid unlocking accounts with enabled external RPC #338

Conversation

uprendis
Copy link
Contributor

@uprendis uprendis commented Jul 7, 2022

  • fix initialization of ExtRPCEnabled var so that unlocking of accounts is forbidden unless --allow-insecure-unlock argument is added

@uprendis uprendis requested review from hadv, cyberbono3 and rus-alex July 7, 2022 19:04
@uprendis uprendis requested a review from andrecronje as a code owner July 7, 2022 19:04
@@ -269,7 +270,7 @@ func newService(config Config, store *Store, blockProc BlockProc, engine lachesi
}

// create API backend
svc.EthAPI = &EthAPIBackend{config.ExtRPCEnabled, svc, stateReader, txSigner, config.AllowUnprotectedTxs}
svc.EthAPI = &EthAPIBackend{false, svc, stateReader, txSigner, config.AllowUnprotectedTxs}
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that we intend to init this with true? otherwise, it still work like before

Suggested change
svc.EthAPI = &EthAPIBackend{false, svc, stateReader, txSigner, config.AllowUnprotectedTxs}
svc.EthAPI = &EthAPIBackend{true, svc, stateReader, txSigner, config.AllowUnprotectedTxs}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's initialized by SetExtRPCEnabled later

Copy link
Contributor

@rus-alex rus-alex left a comment

Choose a reason for hiding this comment

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

Note, incompatible with previous dumped configs:
TOML config file error: field corresponding to "ExtRPCEnabled" is not defined in gossip.Config.

@@ -39,6 +39,11 @@ type EthAPIBackend struct {
allowUnprotectedTxs bool
}

// SetExtRPCEnabled updates extRPCEnabled
func (b *EthAPIBackend) SetExtRPCEnabled(v bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Setter is not needed, just assign it directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to highlight that this variable may be initialized after object creation

Copy link
Contributor

@cyberbono3 cyberbono3 left a comment

Choose a reason for hiding this comment

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

lgtm

@uprendis uprendis merged commit 0a4e16d into Fantom-foundation:develop Jul 12, 2022
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.

4 participants