Test what happens if we call whitelisted condition but with wrong func selector

[hilmarx]

I had a weird case in the alpha UI, where I still used an old function name to encode the condition ("reached"), but in gelatoCore, the condition still passed, even though it could not have returned "OK"

Write following test case with the TimeStateful Condition:

• Create interface with human readable ABI IFace = new utils.Interface([abi]);

• encode data with in incorrect function name

  const conditionPayloadWithSelector = iFace.encodeFunctionData(
    functionname,
    "reached(uint256)"
  );

=> Check if the condition still returns "OK" / goes unnoticed in gelato Core.

[hilmarx]

Update, I think I know what went wrong.

We are always calling the ok(bytes memory _data) function on conditions from gelatoCore, which decodes the data and cuts of the function selector. Hence we can actually encode the data with the wrong function selector and have no problems.

HOWEVER, I was actually passing a uint256 to the ConditionTimeStateful checkRefTime func, but because we decoded the payload in the ok function, we automatically typecasted the encoded uint256 to an address, which got decoded to a useless address which was weird and simply returned "OK", because its state is 0.

Verdict: There will probably arise some issues with our current a) call ok(), b) decode payload and c) see if it returns "OK" approach, as decoding can typecast the encoded values without throwing errors. ⚠️⚠️⚠️

[gitpusha]

But why did you pass a uint256 to the checkRefTime func in the first place? That function takes an address as input, right?

I think the overarching theme here is that dapp developers have to get their encodings right. There is no good workaround. If you encode the wrong type in the payload for a function, unexpected stuff will happen. But it's not really unexpected since you did the encoding wrong. If you do the encoding wrong, you should expect weird stuff to happen.

[gitpusha]

Fazit:

1. Fact: Dapp developers must get their encodings spot-on.

=> We should investigate into what tools we can recommend/build, in order to make it easy for them to get this right.

2. Worrying: Solidity does not revert, if they encoded wrong. Example: uint256 encoded but fn takes address. abi.decode did not revert.

=> This is really not nice. If dapp developers get their encodings wrong, they should at least only have to pay for a revert. But this means that the tx might be triggered even though the user condition was not actually there.

=> Here we must consult with chriseth and find the best way to have some sort of type safety for our encodings/decodings on the relevant functions. Up until here, I had assumed that abi.decode does not make implicit typecasts, but would immediately revert if a wrong type was encoded.