Fix contract-call mode

[praetoriansentry]

Description

I think that gas estimation shouldn't be required at this level. It's done within the library if the GasLimit is NOT specified. The fact that we're manually calling it here is breaking the --gas-limit flag which is needed for some of the tests that I'm running

[leovct]

While it works great for calls sent with --call-only, all the other calls that actually send the transaction will revert with an intrinsic gas too low error.

For example, this one, without --call-only, doesn't work.

$ go run main.go loadtest --verbosity 700 --mode contract-call --contract-address 0x6fda56c57b0acadb96ed5624ac500c0429d59429 --calldata $(cast calldata "storeEther()") --eth-amount 10 --contract-call-payable 
9:43AM TRC Starting logger in console mode
9:43AM INF Starting Load Test
9:43AM INF Connecting with RPC endpoint to initialize load test parameters
9:43AM TRC Retrieved current gas price gasprice=1875175000
9:43AM TRC Retrieved current gas tip cap gastipcap=2750350000
9:43AM TRC Current Block Number blocknumber=1
9:43AM TRC Current account balance balance=1010000000000000000000000000
9:43AM DBG Eip-1559 support detected
9:43AM TRC Detected Chain ID chainID=1337
9:43AM TRC Params Input Params={"AdaptiveBackoffFactor":2,"AdaptiveCycleDuration":10,"AdaptiveRateLimit":false,"AdaptiveRateLimitIncrement":50,"BatchSize":999,"ByteCount":1024,"CallOnly":false,"CallOnlyLatestBlock":false,"ChainID":1337,"ChainSupportBaseFee":true,"Concurrency":1,"ContractAddress":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","ContractCallData":"0xa7cc190a","ContractCallPayable":true,"ContractETHAddress":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","CurrentBaseFee":1000000000,"CurrentGasPrice":1875175000,"CurrentGasTipCap":2750350000,"CurrentNonce":0,"DelAddress":null,"ECDSAPrivateKey":{"Curve":{"B":7,"BitSize":256,"Gx":55066263022277343669578718895168534326250603453777594175500187360389116729240,"Gy":32670510020758816978083085130507043184471273380659243275938904335757337482424,"N":115792089237316195423570985008687907852837564279074904382605163141518161494337,"P":115792089237316195423570985008687907853269984665640564039457584007908834671663},"D":30175782965061331201157137042014122781558886384910785332752666033589853463034,"X":36405839969746785076083915005678656647806951607600131854360360806126023484467,"Y":33607126158016649181688023362411565996635601901352589004706892696521135399893},"ERC20Address":"","ERC721Address":"","EthAmountInWei":10,"ForceContractDeploy":false,"ForceGasLimit":0,"ForceGasPrice":0,"ForcePriorityGasPrice":0,"FromETHAddress":"0x85da99c8a7c2c95964c8efd687e95e632fc533d6","Function":1,"Iterations":1,"LegacyTransactionMode":false,"LtAddress":"","Mode":13,"Modes":["contract-call"],"MultiMode":false,"ParsedModes":[13],"PrivateKey":"42b6e34dc21598a807dc19d7784c71b2a7a01f6480dc6f58258f78e539f1a1fa","RPCUrl":"http://localhost:8545","RateLimit":4,"RecallLength":50,"Requests":1,"Seed":123456,"SendAmount":10000000000000000000,"SendOnly":false,"ShouldProduceSummary":false,"SteadyStateTxPoolSize":1000,"SummaryOutputMode":"text","TimeLimit":-1,"ToAddress":"0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF","ToETHAddress":"0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef","ToRandom":false}
9:43AM DBG Starting main load test loop currentNonce=0
9:43AM TRC Starting Thread routine=0
9:43AM TRC Finished starting go routines. Waiting..
9:43AM DBG Updating gas prices cachedBlockNumber=1 cachedGasTipCap=1875175000 cachedgasPrice=1875175000
9:43AM TRC Contract call data tx={"accessList":[],"chainId":"0x539","gas":"0x0","gasPrice":null,"hash":"0x2f7da074496ba8ae21a5508558670ddc8dbca544bd438d8fbd9fcc2145da2401","input":"0xa7cc190a","maxFeePerGas":"0x6fc4e658","maxPriorityFeePerGas":"0x6fc4e658","nonce":"0x0","r":"0x0","s":"0x0","to":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","type":"0x2","v":"0x0","value":"0x8ac7230489e80000","yParity":"0x0"}
9:43AM ERR Recorded an error while sending transactions error="intrinsic gas too low" nonce=0 request time=0
9:43AM TRC Request mode=loadTestModeContractCall nonce=0 request=0 routine=0
9:43AM DBG Finished main load test loop lastNonce=1 startNonce=0
9:43AM DBG Waiting for remaining transactions to be completed and mined
9:43AM TRC Not all transactions have been mined. Waiting currentNonceForFinalBlock=0 endNonce=1 prevNonceForFinalBlock=0
9:43AM TRC Retrying... Remaining Attempts=19
...
9:44AM ERR There was an issue waiting for all transactions to be mined error="waited for 20 attempts for the transactions to be mined"
...
9:44AM INF Num errors numErrors=1
9:44AM INF Finished

And this one, with --call-only, works well.

$ go run main.go loadtest --verbosity 700 --mode contract-call --contract-address 0x6fda56c57b0acadb96ed5624ac500c0429d59429 --calldata $(cast calldata "getNumber()(uint)") --call-only
9:58AM TRC Starting logger in console mode
9:58AM INF Starting Load Test
9:58AM INF Connecting with RPC endpoint to initialize load test parameters
9:58AM TRC Retrieved current gas price gasprice=1768196701
9:58AM TRC Retrieved current gas tip cap gastipcap=2536393402
9:58AM TRC Current Block Number blocknumber=2
9:58AM TRC Current account balance balance=1009999999998714898965800000
9:58AM DBG Eip-1559 support detected
9:58AM TRC Detected Chain ID chainID=1337
9:58AM TRC Params Input Params={"AdaptiveBackoffFactor":2,"AdaptiveCycleDuration":10,"AdaptiveRateLimit":false,"AdaptiveRateLimitIncrement":50,"BatchSize":999,"ByteCount":1024,"CallOnly":true,"CallOnlyLatestBlock":false,"ChainID":1337,"ChainSupportBaseFee":true,"Concurrency":1,"ContractAddress":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","ContractCallData":"0xf2c9ecd8","ContractCallPayable":false,"ContractETHAddress":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","CurrentBaseFee":875175000,"CurrentGasPrice":1768196701,"CurrentGasTipCap":2536393402,"CurrentNonce":1,"DelAddress":null,"ECDSAPrivateKey":{"Curve":{"B":7,"BitSize":256,"Gx":55066263022277343669578718895168534326250603453777594175500187360389116729240,"Gy":32670510020758816978083085130507043184471273380659243275938904335757337482424,"N":115792089237316195423570985008687907852837564279074904382605163141518161494337,"P":115792089237316195423570985008687907853269984665640564039457584007908834671663},"D":30175782965061331201157137042014122781558886384910785332752666033589853463034,"X":36405839969746785076083915005678656647806951607600131854360360806126023484467,"Y":33607126158016649181688023362411565996635601901352589004706892696521135399893},"ERC20Address":"","ERC721Address":"","EthAmountInWei":0.001,"ForceContractDeploy":false,"ForceGasLimit":0,"ForceGasPrice":0,"ForcePriorityGasPrice":0,"FromETHAddress":"0x85da99c8a7c2c95964c8efd687e95e632fc533d6","Function":1,"Iterations":1,"LegacyTransactionMode":false,"LtAddress":"","Mode":13,"Modes":["contract-call"],"MultiMode":false,"ParsedModes":[13],"PrivateKey":"42b6e34dc21598a807dc19d7784c71b2a7a01f6480dc6f58258f78e539f1a1fa","RPCUrl":"http://localhost:8545","RateLimit":4,"RecallLength":50,"Requests":1,"Seed":123456,"SendAmount":1000000000000000,"SendOnly":false,"ShouldProduceSummary":false,"SteadyStateTxPoolSize":1000,"SummaryOutputMode":"text","TimeLimit":-1,"ToAddress":"0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF","ToETHAddress":"0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef","ToRandom":false}
9:58AM DBG Starting main load test loop currentNonce=1
9:58AM TRC Starting Thread routine=0
9:58AM TRC Finished starting go routines. Waiting..
9:58AM DBG Updating gas prices cachedBlockNumber=2 cachedGasTipCap=1768196701 cachedgasPrice=1768196701
9:58AM TRC Contract call data tx={"accessList":[],"chainId":"0x539","gas":"0x0","gasPrice":null,"hash":"0xd0dabaf0e3cd1350bd24f043acd95ffd8d1a5679ff2f2797d129dd5b83861bdb","input":"0xf2c9ecd8","maxFeePerGas":"0x69648a5d","maxPriorityFeePerGas":"0x69648a5d","nonce":"0x1","r":"0x0","s":"0x0","to":"0x6fda56c57b0acadb96ed5624ac500c0429d59429","type":"0x2","v":"0x0","value":"0x0","yParity":"0x0"}
9:58AM TRC Request mode=loadTestModeContractCall nonce=1 request=0 routine=0
9:58AM DBG Finished main load test loop lastNonce=2 startNonce=1
9:58AM DBG Waiting for remaining transactions to be completed and mined
9:58AM INF CallOnly mode enabled - blocks aren't mined
9:58AM INF Finished

Instead of removing the gas estimation, I would only use it if the call is not done using ---call-only (meaning we want to send a tx) and if the gas limit is not set. I tested the implementation and it works well for regular calls and calls with --call-only. @praetoriansentry, can you test if it still fails with your tests?

It looks like I can't suggest code modification on deleted lines in Github UI so I'll just write the code here.

// Override the gas limit only when sending a transaction and the user has not set the gas limit.
if !*ltp.CallOnly && *ltp.ForceGasPrice == 0 {
	estimateInput := ethereum.CallMsg{
		From:  *ltp.FromETHAddress,
		To:    to,
		Value: amount,
		Data:  calldata,
	}
	tops.GasLimit, err = c.EstimateGas(ctx, estimateInput)
	if err != nil {
		log.Error().Err(err).Msg("Unable to estimate gas for transaction")
		return
	}
}

[praetoriansentry]

Yeah I guess since this is implemented without any binding, the gas estimation has to be done explicitly. For some reason the contracts that I was testing were constantly failing with the implementation in the main branch (i.e. gas estimation was failing). In that case, it might be convenience for testing to fail with the max value rather than erroring out. But for now I think we can just require users to set the --gas-limit like what I was doing.

I think you would still want to estimate gas even when the --call-only flag is specified. Otherwise the behavior will be different in terms of execution.

I think that the specific implementation of the load test function probably doesn't check the ForcedGasPrice it should rely on the implementation of configureTransactOpts (ideally). In that case I think tops.GasLimit should be set already according to the forced gas price. If it's 0 then we can estimate. That aligns with the implementation from the createDynamicTx function

[IdrisHanafi]

utACK

I see, so the original implementation that I had ignores the --gas-limit flag. Now, if the user sets a --gas-limit other than the default (0), then it doesn't EstimateGas and just uses the user's set value.

The main reason why I had to manually call GasLimit there is because the calldata is going to influence the gas cost based on the calldata size. So I needed a way to quickly calculate and set that value within the created tx.

[praetoriansentry]

Yeah that sounds right. The call data represents the function being called and the arguments. Even if the size of the call data is constant, it could be calling different methods with different arguments which is definitely going to change the amount of gas needed to execute. The reason why I have to specify the gas-limit manually is because estimate gas doesn't always work. In my case, the functions alter their behavior based on the gas-limit, so it's impossible to estimate the gas limit. The original implementation was always estimating and therefore always failing.

I was confused at first because the other functions estimate gas within the context of the bound contract. Since this is an arbitrary contract, it makes sense that we have to do an estimate. I had overlooked that difference.

[leovct]

lgtm :)

[IdrisHanafi]

fyi: I branched off from this branch to create this PR: #178

[leovct]

Closing this PR as it has already been merged in #178