There should be a way to test overwriting pending transactions

[matthewjablack]

With the current functionality of Ganache Core, if you try to broadcast a transaction with a higher fee that has already been broadcast with the same nonce, you will receive the following error the tx doesn't have the correct nonce. account has nonce of: 1 tx has nonce of: 0.

This is because it verifies the nonce itself before running against the vm https://github.com/trufflesuite/ganache-core/blob/283cbeca1cf9f9c598845a00baaa331afc621596/lib/statemanager.js#L965

ganache-core does not currently support a mempool, although this WIP refactor by @davidmurdoch does support a mempool to some degree https://github.com/trufflesuite/ganache-core/blob/a4e8989efc301b28d094082297dfbb1c4b25a744/src/ledgers/ethereum/components/transaction-pool.ts#L80-L87

A feature like this is incredibly important for DApp developers. Especially ones creating time sensitive DApps that need to ensure parties can overwrite pending transactions.

[jjgonecrypto]

100% this feature is imperative for testing complicated pending and cancelling scenarios.

[jacko125]

upvote +1

[Talvish]

upvote +1

[davidmurdoch]

Pending transaction support (with geth-like tx replacement mechanisms) is slated for the next major release.

[monokh]

@davidmurdoch I see that the PR for fixing some of this has been merged. Is this feature now available on the next tag? I would be glad to try this for our tests and feedback on it's compatibility with the geth dev mode behaviour.

[davidmurdoch]

@monokh it's not yet released. I'm close to cutting an alpha/internal release (it won't include forking). It also won't include the ability to use the "pending" block tag. If you can't still run your tests even with those caveats let me know and I'll tag you when I create the release.

[monokh]

Thanks @davidmurdoch That still covers my use case. The main feature we required was correct handling of pending nonces, such that it is possible to bump transaction fee.

[davidmurdoch]

@monokh Awesome, initially the price bump % amount will be hard coded to 10%; will this hard-coded limitation work for you?

[davidmurdoch]

@monokh you can try it out via npm install ganache@internal or for global cli use: npm install ganache@internal -g.

This version is pretty much guaranteed to have new bugs and regressions. And here is a list of known changes and breaking changes: #657

One of the biggest problems with the cli is that I forgot to add the legacyInstamine flag, which needs to be true for tests that immediately check for a tx receipt after receiving the tx hash, as the receipt may return null. This won't be a problem if your tests already use geth, as this is how geth works already.

I'll probably cut a few more internal release this week, so you can expect lots of changes coming soon.

One final caveat: don't spend much time updating your tests as this release's API is not considered stable.

If you do end up trying this release let me know how it goes, even if it's a catastrophic failure!

[monokh]

@davidmurdoch The fee updating tests worked seamlessly when migrating from geth to ganache!
I did have some tests that have started to fail but I believe these are unrelated to your changes. More to do with the differences with geth.

[monokh]

@davidmurdoch Actually I think i have found a regression in the latest version.
eth_getCode returns empty, even with latest block.

[davidmurdoch]

The fee updating tests worked seamlessly when migrating from geth to ganache!

That is great to hear, @monokh!

I did have some tests that have started to fail but I believe these are unrelated to your changes. More to do with the differences with geth.

For many aspects I'd like to try to align more with geth, so if it's not too hard to pin point where we differ I'd greatly appreciate if you could provide more info.

Actually I think I have found a regression in the latest version. eth_getCode returns empty, even with latest block.

I'll try to reproduce now, please send reproduction steps if you have them!

Thanks again!

[monokh]

False alarm. The actual issue was that the contract was not deployed. Tx receipt was returned with status: 0 and a contract address (should it?)

But the inconsistency with geth is that the transaction gas limit seems to be hardcoded to 90000 when not supplied in the transaction. On geth, I believe the node is appending an estimate.
Example:

await web3.eth.sendTransaction({ from: accounts[0], data: '0x608060405234801561001057600080fd5b5060408051678ac7230489e800008152905133916000917fddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef9181900360200190a3336000908152602081905260409020678ac7230489e80000905561055b8061007a6000396000f3fe608060405260043610610087577c0100000000000000000000000000000000000000000000000000000000600035046306fdde03811461008c578063095ea7b31461011657806323b872dd14610163578063313ce567146101a657806370a08231146101d157806395d89b4114610216578063a9059cbb1461022b578063dd62ed3e14610264575b600080fd5b34801561009857600080fd5b506100a161029f565b6040805160208082528351818301528351919283929083019185019080838360005b838110156100db5781810151838201526020016100c3565b50505050905090810190601f1680156101085780820380516001836020036101000a031916815260200191505b509250505060405180910390f35b34801561012257600080fd5b5061014f6004803603604081101561013957600080fd5b50600160a060020a0381351690602001356102d6565b604080519115158252519081900360200190f35b34801561016f57600080fd5b5061014f6004803603606081101561018657600080fd5b50600160a060020a0381358116916020810135909116906040013561033c565b3480156101b257600080fd5b506101bb6103ab565b6040805160ff9092168252519081900360200190f35b3480156101dd57600080fd5b50610204600480360360208110156101f457600080fd5b5035600160a060020a03166103b0565b60408051918252519081900360200190f35b34801561022257600080fd5b506100a16103c2565b34801561023757600080fd5b5061014f6004803603604081101561024e57600080fd5b50600160a060020a0381351690602001356103f9565b34801561027057600080fd5b506102046004803603604081101561028757600080fd5b50600160a060020a038135811691602001351661040f565b60408051808201909152600a81527f546f6b656e205465737400000000000000000000000000000000000000000000602082015281565b336000818152600160209081526040808320600160a060020a038716808552908352818420869055815186815291519394909390927f8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925928290030190a350600192915050565b600160a060020a038316600090815260016020908152604080832033845290915281205482111561036c57600080fd5b600160a060020a03841660009081526001602090815260408083203384529091529020805483900390556103a184848461042c565b5060019392505050565b601281565b60006020819052908152604090205481565b60408051808201909152600481527f5357415000000000000000000000000000000000000000000000000000000000602082015281565b600061040633848461042c565b50600192915050565b600160209081526000928352604080842090915290825290205481565b600160a060020a038216151561044157600080fd5b600160a060020a03831660009081526020819052604090205481111561046657600080fd5b600160a060020a038216600090815260208190526040902054818101101561048d57600080fd5b600160a060020a03808316600081815260208181526040808320805495891680855282852080548981039091559486905281548801909155815187815291519390950194927fddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef929181900390910190a3600160a060020a0380841660009081526020819052604080822054928716825290205401811461052957fe5b5050505056fea165627a7a72305820db460d87e53e94fdd939b99d2a07ceb235e8a2ce62f7d320cd34a12c1c613a860029' })

Error: The contract code couldn't be stored, please check your gas limit.

I think this has always been the issue with Ganache though.

[davidmurdoch]

@monokh I was unable to reproduce myself, but on the plus side, I nearly doubled the number of eth_getCode tests in the process (PR here: #667)

Tx receipt was returned with status: 0 and a contract address (should it?)

No, I don't believe it should. I'll look into why this is happening.

But the inconsistency with geth is that the transaction gas limit seems to be hardcoded to 90000 when not supplied in the transaction. On geth, I believe the node is appending an estimate.

The default transaction gas limit is will be configurable via cli eventually. With that said, I thought this behavior was in-line with geth's. I'll have to do more digging here.

Thanks!

[davidmurdoch]

@monokh it seems that geth changed from using the default 90000 transaction gas limit to using its own estimate in April of last year: ethereum/go-ethereum#19508 (comment)

[davidmurdoch]

RE:

No, I don't believe it should. I'll look into why this is happening.

@monokh I just checked geth's response for failed contract creation and it aligns with ours.

Here are the steps I took to reproduce on geth:

• Start up geth in dev mode on port 8545
• use the dev account to upload a contract, but don't supply enough gas
• get receipt; status is "0x0" and contractAddress is non-null.

curl commands for reproduction (update params where necessary):

$ curl http://localhost:8545 -H 'Content-Type: application/json' --data '{"id": 1, "method": "eth_sendTransaction", "params":[ {"from": "0xb132A8a07e9E2e286AcE35110c294220d30E1e98", "gas": "0x10000", "data": "0x6080604052348015600f57600080fd5b50603f80601d6000396000f3fe6080604052600080fdfea2646970667358221220c7bc579e8a7331a8a5ca62d82e464a95365884707bfa3344d5810466213946b864736f6c63430007040033"} ]}'
{"jsonrpc":"2.0","id":1,"result":"0xfb83518ef1f2a26cc92407a9699c91bb79fdefa426ebddf0169895c0ea8c5dc3"}


$ curl http://localhost:8545 -H 'Content-Type: application/json' --data '{"id": 1, "method": "eth_getTransactionReceipt", "params":[ "0x58b6a55a1f987950d0229b8d359468b37afecece49a88cb40c18b689c665f326" ]}'
{"jsonrpc":"2.0","id":1,"result":{"blockHash":"0x3e2c7726d464898087a4167f4e704fa72d34d52ad45aa7568852d7848d302d0b","blockNumber":"0x1","contractAddress":"0xc134e0d5e31d0c362446c76931c74c15eae4f1fb","cumulativeGasUsed":"0x10000","from":"0x6293d184b8e1c893dd6be0315d8177b4dec1434b","gasUsed":"0x10000","logs":[],"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","status":"0x0","to":null,"transactionHash":"0xfb83518ef1f2a26cc92407a9699c91bb79fdefa426ebddf0169895c0ea8c5dc3","transactionIndex":"0x0"}}

We'll talk internally if we want to change our default behavior in this next major release, I imagine the answer is "Yes!", so I'd expect this to change in the near future.

[davidmurdoch]

Thinking about it more, making this change will slow down eth_sendTransaction requests that omit gas/gasLimit by maybe 40-60% (as gas estimation must be run at least twice for every estimate, and each run is slower than just running the transaction in the first place).

This performance difference isn't really a concern for geth, as geth needs to worry about mining a block in ~15 seconds, not running a new transaction in 20 milliseconds. But for ganache, this could theoretically double the execution time of some test suites. If we do change anything, I'm now leaning towards making this behavior opt-in.

[monokh]

@davidmurdoch Not estimating gas in the client sounds right if you are worried about the performance. I would ask though in that case for the default gas limit to be configurable. At the moment I have modified the tests to supply estimateGas.

That was the only change that was required and everything else passed! Looking forward to seeing it in prod :)

[davidmurdoch]

@monokh I've released [email protected] which includes a new cli option: defaultTransactionGasLimit, which you can set to the string estimate to align the behavior with geth's (ganache --defaultTransactionGasLimit estimate)`.

[davidmurdoch]

done in [email protected]