flaky stress test

[hackaugusto]

the cache was introducing flakyness in the tests, this happened with
tests that are long running but don't send many transactions, which
makes the block gaslimit decrease significantly after the private
blockchain fixture setup is done

[palango]

LGTM

[LefterisJP]

@hackaugusto Very nicely spotted.

But wouldn't it be better to somehow remove the gas limit cache only for the stress test and keep it in the actual production code? Isn't it useful in production?

[hackaugusto]

Isn't it useful in production?

I don't think so, this will always be a problem when the chain is under congestion. If the idea is to save RPC calls then we should first measure the performance impact, and if measured to be worth while add a cache on a per block basis.

Edit: In general I think we should only use caches for memoizatoin, otherwise it introduces data races like this one. i.e. the gasprice cache should go too.

[LefterisJP]

If the idea is to save RPC calls then we should first measure the performance impact, and if measured to be worth while add a cache on a per block basis.

Completely agree on the general idea of optimizing only if a bottleneck is found.

I don't think so, this will always be a problem when the chain is under congestion.

I am not sure I understand why blockchain congestion plays a part here. Can you elaborate a bit?

The TTL cache essentially waits 600 seconds until making a new RPC call. In real blockchain scenarios 600 seconds is about 42 blocks. The gas limit can not change significantly in the ethereum mainnet in that small amount of blocks.

[andrevmatos]

As @hackaugusto proposed in #2623 , the actual solution is to not use last block gas as gaslimit, but instead either hardcode known limits or properly estimating gas.
I just don't agree with optimizing only if measured and a bottleneck is found, but instead, wherever makes sense/we need to contact a partner, a service provider, rpc node or general IO, if it's possible to easily cache these things, e.g. if we still need to contact the eth node to retrieve the last block used gas, there's no point in doing it more than once every block, even before benchmarking the impact on the Raiden node, as we can't measure beforehand the impact of millions of IoT nodes querying Infura's servers or thousands of users with nodes on light mode and only a handful of lightserves available, so IMHO IO/blockchain/matrix/network in general should be handled with parsimony everywhere by default.

[LefterisJP]

yeah properly estimating gas sounds good