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

feat: Add methods to modify account #2337

Conversation

rmeissner
Copy link
Contributor

@rmeissner rmeissner commented Feb 10, 2022

Implements #1889

Adds:

  • evm_setAccountBalance
  • evm_setAccountCode
  • evm_setAccountStorageAt

@rmeissner
Copy link
Contributor Author

rmeissner commented Feb 10, 2022

@davidmurdoch I was wondering how the api documentation is generated and if there is a need to manually adjust it (e.g. https://github.com/trufflesuite/ganache/blob/develop/docs/typedoc/api.json)

Also I would also try to add logic to set the storage and to set the code for an account. Should this happen in different PRs?
I added all the different rpc methods to this PR for now, let me know if I should split them.

@rmeissner rmeissner marked this pull request as ready for review February 10, 2022 13:04
@rmeissner rmeissner changed the title feat: Add evm_setAccountBalance feat: Add methods to modify storage Feb 10, 2022
@rmeissner rmeissner changed the title feat: Add methods to modify storage feat: Add methods to modify account Feb 10, 2022
@davidmurdoch
Copy link
Member

Wow, at first glance this looks very thorough!

The docs are auto-generated as part of the release and end up here (for now). Note that these docs are actually just a proof of concept and will likely be rewritten.

@rmeissner
Copy link
Contributor Author

Cool :)

Let me know if I should adjust any of the method names (or anything else).

I am not sure why some of the tests are failing :/

Also another thing I realized is that src/packages/ganache/npm-shrinkwrap.json and src/chains/filecoin/filecoin/package-lock.json have changes (I assume from running npm i). Not sure if this is ok, else I would revert them.

@rmeissner
Copy link
Contributor Author

@davidmurdoch is there any way to push this forward from my side? I want to use Ganache v7 to simulate Safe transactions and for this I require this functionality :D

@MicaiahReid
Copy link
Contributor

MicaiahReid commented Feb 15, 2022

@rmeissner I'll review/test this today and we can hopefully get it merged this week!

@davidmurdoch davidmurdoch self-requested a review February 15, 2022 17:26
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

Looks great! Can you drop the filecoin package-lock.json changes? Once that is done this should be good to go on my end!

Excellent work!

@davidmurdoch
Copy link
Member

Oh, we have a flaky test and a couple of tests that like like to timeout, so don't worry about the tests failures here.

Copy link
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Great work and thanks for the PR! I've requested a few simple changes.

One thing we (@rmeissner, @davidmurdoch, whoever is interested) should probably discuss is data validation for these methods. Currently, if you send bad data for the balance/code/storage, for example "0xw", it will always respond true. Also, if there is some valid data, it will update the balance/code/storage up to the bad data. For example, setting balance to "0xab;lkjf" will set the balance to 0xab. This is also true for evm_setAccountNonce, not just these new methods.

src/chains/ethereum/ethereum/RPC-METHODS.md Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/RPC-METHODS.md Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
rmeissner and others added 4 commits February 15, 2022 19:10
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
@rmeissner
Copy link
Contributor Author

Yeah, I followed evm_setAccountNonce, but I agree some data validation would be nice. Maybe something for a separate PR, but I could look into this.

@davidmurdoch davidmurdoch merged commit c0228bc into trufflesuite:develop Feb 18, 2022
@davidcallanan
Copy link

Hello, any idea when this is being officially released?

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