Adds deterministic testing for Ethereum

[mikewcasale]

Handles issue #398

[mikewcasale]

Note that prior to changing "Draft" status on this PR, I still need to add the corresponding updates to the Github Actions, test it, &etc. In the meantime however it would be helpful to get your feedback on the core code changes.

[barakman]

Why such a different output structure for eth vs for non-eth?
Wouldn't that cause havoc on the caller function?

[mikewcasale]

It does not cause havoc, as the implementation fundamentally works as expected. However, to a large extent my efforts here were to cleanup and improve upon @NIXBNT 's original solution, while not altering the underlying logic to any great extent - thus, I would defer to Nick on your first question...

[sklbancor]

I would concur with Barak here that returning a length 2 / length 3 list that really should be a dict or dataclass can be problematic; not a show stopper given that it fits in the current design but should probably be raised as an overall cleanup issue

[mikewcasale]

The return value of the faucet_params( ) method is immediately used to make a request to Tenderly, as you can see in the following code:

params = token_balance.faucet_params(wallet_address=self.pool_address)
method_name = RPCEndpoint(
    "tenderly_setBalance"
    if token_balance.token.is_eth
    else "tenderly_setErc20Balance"
)
w3.provider.make_request(method=method_name, params=params)

Thus, I refer you to the Tenderly documentation for reference, so you can see the expected structure of ETH vs ERC20 requests:

• tenderly_setBalance - https://docs.tenderly.co/devnets/advanced/custom-rpc-methods#tenderly_setbalance

• tenderly_setErc20Balance - https://docs.tenderly.co/devnets/advanced/custom-rpc-methods#tenderly_seterc20balance

So, while I can see your point of concern, in this case, the reason behind it is strictly 3rd party api's, and IMO changing the handling would add complexity and offer no benefit, and possibly introduce bugs in the process.

[sklbancor]

I would concur with Barak here that returning a length 2 / length 3 list that really should be a dict or dataclass can be problematic; not a show stopper given that it fits in the current design but should probably be raised as an overall cleanup issue

[barakman]

Best to maintain all floating point values as strings, and convert them to Decimal in the code, so as to avoid accuracy loss.

[mikewcasale]

can you be more specific about which values you're referring to? as of now, the tests pass in the sense of finding the expected values, and what you're suggesting seems like it could mean converting all float and int types to Decimal, which is a fairly risky effort in terms of introducing bugs -- which in this case means attempting to fix what isn't broken

[barakman]

I'm saying that in data files (JSONs CSVs, etc), we should generally represent non-integer values with strings.