Add tests for gas estimate for simple value transfer between accounts

[davidmurdoch]

Also look into adding integration tests with MetaMask.

[wbt]

This kind of simple transfer requires more up-front gas in Ganache 1.2.2. than in 1.2.1. That can be a breaking change if attempting to transfer all or close to all of an account's value.

[davidmurdoch]

I believe that is the intended behavior. If you transfer all of an account's value, setting it to zero, a 15,000 gas refund is given immediately after the transaction is complete. Ganache had been underestimating the gas required, causing OOG errors when running a transaction that set storage to 0 when supplying only the estimated gas.

Because we now add in the potential refund amount to the gas estimate Ganache may provide gas estimates that are on the high side. We are working on making the estimate more even more accurate which is will be tracked here: #147

Has the actual gasUsed changed in your tests, or just the estimate?

[wbt]

I am not sure that the actual gasUsed has changed; I didn't track that. An experiment could answer the question. I would not expect any change in the "close to all" case.

I just noticed an undocumented breaking change on a software update that was just supposed to be minor bugfixes, thought that was an issue deserving a fix, and this recent Issue looked quite relevant for the topic. I thought the motivation to add tests and/or apply the "priority-high" label might have been because there was an undocumented breaking change with the upgrade, but this was not explicit in the lack of description for the issue. So I introduced it more explicitly in the discussion.

[davidmurdoch]

Sorry if I sounded confrontational in any way in my last comment. Thank you for trying to clear up this vague issue!

This specific GitHub issue is regarding a hole in our testing that was the root cause of our latest ganache-core HotFix release. While we do test account to account transfers, we currently don't have tests that estimate the gas required for this same type of transfer.

I don't think there are any breaking changes with regards to gas estimation on simple value transfers between accounts in the Ganache latest release. Gas estimations may change and evolve in future releases as we fine tune the estimates. I'll chat with the team as to whether or not we should consider gas estimate calulation changes as breaking changes in our future ganache-core and ganache-cli semver releases. Thanks again for bringing this up!

[wbt]

It didn't seem confrontational to me.
I apparently guessed wrong at the motivation for this Issue, and will open another. Look for the reference to appear just below.
Thanks for your work!

[eshaben]

Test has been created for this in PR #204! Closing the issue.