[copied from exact same reasoning with execution code-checking]
I was inspired to do this because, when disabling the slither check for the staticcall when calling the callback verifier, I realized it's not clear from the same contract that we have already checked for contract code existence when setting the verifier. This made me feel uneasy, as this contract can then not stand alone and must rely on the higher level contract to safely check for code existence, otherwise the staticcall is unsafe. Keeping this logic in a separate contract seems error-prone to me, as we may remove the check for code existence without immediately realizing the implications of doing so.
For this reason I have organized it as follows:
- Logic/tests relating to proper roles/access control in the main TychoRouter.
- Lower-level logic/tests that check contract validity before setting the callback verifier in the CallbackVerificationDispatcher
- This is not so relevant for security, but it would sabotage our performance if an executor was wrongly removed, so it's worth it to know every time this happens.
I was inspired to do this because, when disabling the slither check for the delegatecall when calling the swap executor, I realized it's not clear from the same contract that we have already checked for contract code existence when setting the executor. This made me feel uneasy, as this contract can then not stand alone and must rely on the higher level contract to safely check for code existence, otherwise the delegatecall is unsafe. Keeping this logic in a separate contract seems error-prone to me, as we may remove the check for code existence without immediately realizing the implications of doing so.
For this reason I have organized it as follows:
- Logic/tests relating to proper roles/access control in the main TychoRouter.
- Lower-level logic/tests that checks contract validity before setting the executor in the SwapExecutionDispatcher
- low level calls are fine, since we are checking for success, and we have already checked for contract existence when setting swap executors
- dead-code is silenced
- fix solidity version
This was running the test that should be skipped in CI and it is flagged as
#[cfg_attr(not(feature = "fork-tests"), ignore)]
--- don't change below this line ---
ENG-4063 Took 6 minutes
- Add data validations similar to the ones done in the Permit2 SDK
- Fix Domain main (!!!) It's Permit2 not Permit
- Return the whole function signature data (owner, permit_single, signature) encoded
test improvements:
- Don't compare the timestamps, this was making the test fail sometimes and pass other times
- Add a test to run on an anvil fork and actually call permit2 contract to double check the encoded data works
misc:
Rename get_allowance_data -> get_existing_allowance
--- don't change below this line ---
ENG-4063 Took 5 hours 19 minutes
Took 11 seconds
- Retrieve allowance data from an RPC
- Encode approvals:
- Create PermitSingle (and PermitDetails)
- Use alloy eip712 to get signing hash on typed data
- Sign data
- Add chrono and alloy eip712 and signer-local to Cargo.toml
--- don't change below this line ---
ENG-4063 Took 1 hour 58 minutes
Took 18 seconds
Took 32 seconds
Took 9 seconds
Took 1 minute
Took 32 minutes
