Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Problem: personal_newAccount don't work #1561

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Problem: personal_newAccount don't work #1561

merged 1 commit into from
Dec 22, 2022

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Dec 19, 2022

Description

fix the internal parameter.

Shall we just deprecated the personal apis, since they are rarely used, and not well tested.


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

fix the internal parameter.
@yihuang yihuang requested a review from a team as a code owner December 19, 2022 03:37
@yihuang yihuang requested review from hanchon and facs95 and removed request for a team December 19, 2022 03:37
@codecov
Copy link

codecov bot commented Dec 19, 2022

Codecov Report

Merging #1561 (f0e7bd6) into main (111042d) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1561   +/-   ##
=======================================
  Coverage   68.63%   68.63%           
=======================================
  Files         103      103           
  Lines        9946     9946           
=======================================
  Hits         6826     6826           
  Misses       2737     2737           
  Partials      383      383           
Impacted Files Coverage Δ
rpc/backend/node_info.go 46.51% <0.00%> (ø)

@facs95
Copy link
Contributor

facs95 commented Dec 22, 2022

I think we probably should - we are not currently using them not am I aware of anybody that does use them. I will merge this PR for now and discuss internally how to properly manage it to make sure we are not affecting any ethermint user.

@facs95 facs95 merged commit 0e366db into evmos:main Dec 22, 2022
@yihuang yihuang deleted the patch-1 branch December 22, 2022 00:59
@fedekunze
Copy link
Contributor

@yihuang thanks for the PR. Could you (or @facs95) add a changelog entry?

MalteHerrmann pushed a commit that referenced this pull request Dec 22, 2022
@MalteHerrmann MalteHerrmann mentioned this pull request Dec 22, 2022
fedekunze added a commit that referenced this pull request Dec 22, 2022
* build(deps): bump github.com/cosmos/cosmos-sdk from 0.46.6 to 0.46.7 (#1551)

Bumps [github.com/cosmos/cosmos-sdk](https://github.com/cosmos/cosmos-sdk) from 0.46.6 to 0.46.7.
- [Release notes](https://github.com/cosmos/cosmos-sdk/releases)
- [Changelog](https://github.com/cosmos/cosmos-sdk/blob/main/CHANGELOG.md)
- [Commits](cosmos/cosmos-sdk@v0.46.6...v0.46.7)

---
updated-dependencies:
- dependency-name: github.com/cosmos/cosmos-sdk
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump github.com/cosmos/ibc-go/v5 from 5.1.0 to 5.2.0 (#1564)

Bumps [github.com/cosmos/ibc-go/v5](https://github.com/cosmos/ibc-go) from 5.1.0 to 5.2.0.
- [Release notes](https://github.com/cosmos/ibc-go/releases)
- [Changelog](https://github.com/cosmos/ibc-go/blob/v5.2.0/CHANGELOG.md)
- [Commits](cosmos/ibc-go@v5.1.0...v5.2.0)

---
updated-dependencies:
- dependency-name: github.com/cosmos/ibc-go/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* make missing key error message during SendTransaction more verbose (#1563)

Co-authored-by: 4rgon4ut <[email protected]>

* debug(app): add flag to disable optimized build for remote debugging (#1549)

Co-authored-by: MalteHerrmann <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Problem: personal_newAccount don't work (#1561)

fix the internal parameter.

* fix(ante): fix reCheckTx gas wanted (#1566)

* fix(abci): fix reCheckTx gas wanted'

* fix(ante): add changelog entry

* fix(cli): fix Ledger signature algorithm verification (#1550)

* fix: update Ledger default algorithm to `EthSecp256k1`

* fix ledger signing algo validation

* changelog

Co-authored-by: Freddy Caceres <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>

* update changelog

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: 4rgon4ut <[email protected]>
Co-authored-by: Tomas Guerra <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: yihuang <[email protected]>
Co-authored-by: Austin Chandra <[email protected]>
Co-authored-by: Freddy Caceres <[email protected]>
@ferjep
Copy link

ferjep commented Dec 27, 2022

Description says

Shall we just deprecated the personal apis, since they are rarely used, and not well tested.

Then what's the right way (or most used) to create an address (RPC or any other API way)? personal_newAccount is the only way I know in Ethereum compatible blockchains and can't find docs about it for ethermint

@yihuang
Copy link
Contributor Author

yihuang commented Dec 27, 2022

Description says

Shall we just deprecated the personal apis, since they are rarely used, and not well tested.

Then what's the right way (or most used) to create an address (RPC or any other API way)? personal_newAccount is the only way I know in Ethereum compatible blockchains and can't find docs about it for ethermint

Just sign tx with a client library, and some key management tool? I use ethsign cli tool for development.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants