Erc7683 allocator

[mgretzke]

Pull Request

Description

Introducing the ERC7683Allocator contract, a fully decentralized on chain allocator that follows ERC7683 as an origin settler.
It is build on top of the SimpleAllocator contract and extends it to follow the IOriginSettler interface.
The contract allows users to distribute their order via an open event during the lock process. There are two ways to distribute the order:

• The open function is only callable by the sponsor directly and it requires them to have previously registered the claim hash in the Compact
• The openFor´ function can be called by anyone. The caller must provide a signature proving the users intents, together with the GaslessCrossChainOrder`. This function can also be called without providing a signature, if the user has previously registered a claim hash in the Compact.

The token allocation is following the SimpleAllocator design, where all the tokens of a specific ID get locked, no matter how big the amount of the claim is. This makes it easier and more efficient to manage the expiration dates.

A topic still in discussion is the resolveFor function. In the current ERC7683 interface, it does not support the signature as an input. This leads to a returned ResolvedCrossChainOrder struct which does not contain the signature. While this can still be used to simulate the outcome on the destination chain, actually using the returned data would mean a filler would lose their reward, since the signature is required to prove the users intents during the claim. While fillers typically would use the correct ResolvedCrossChainOrder struct that was emitted in the open event, it still seems problematic to include a function that potentially returns invalid data.

It also includes changes to the foundry.toml, which are required to successfully compile TheCompact contract which is used for testing.

How Has This Been Tested?

The contract is still in development and neither audited, nor has it been fully tested.

[0age]

Very nice! Left a few comments

[0age]

I think we might just want this to be the fragment, so Mandate mandate)Mandate(uint256 chainId,address tribunal,address recipient,uint256 expires,address token,uint256 minimumAmount,uint256 baselinePriorityFee,uint256 scalingFactor,bytes32 salt)

[mgretzke]

I feel like both can be useful, since this will be the full type string required for the sponsors signature... should I just implement the fragmented return in an additional function, or really replace this one?

[0age]

the HyperlaneTribunal requires that a 64-byte signature be provided in the calldata on the cross-chain message, even if it's not needed; we should probably provide a 64-byte string of 00's just so that fillers can use this directly

[mgretzke]

I have an issue with this:

The sponsors signature might also be empty, for example if the user has registered a claim previously and is using the open function to open the order without a signature. So that would also need to be an empty 64-bytes.

While implementing this change and writing the tests (ensuring both allocator and sponsor signature are an empty 64-bytes, instead of just an empty bytes), I realized that the Compact claim function will fail, if you provide an empty 64-byte string for the sponsors signature. This is because the Compact will check for the length of the sponsors signature, and if it is bigger then 0, it will ignore potential registered claims. It will instead rely on the signature verification / ERC1271 (which will both fail for an empty 64-bates signatures). This is the relevant line:
https://github.com/Uniswap/the-compact/blob/c4776fa3264f7c64b26b5800b5f8d8e651089b0a/src/lib/ClaimProcessorLib.sol#L92

So it seems the correct way with the Compact is to actually provide an empty bytes, instead of an empty 64-byte signature. Should we therefor not rather change this in the HyperlaneTribunal contract?

[0age]

HyperlaneTribunal just expects 64 bytes, but if it's all zeroes it uses an empty string (we could change it there but encoding even an empty dynamic string in solidity is the same amount of calldata!)

[0age]

shouldn't we be checking that the token on the minReceived Output matches the token in the ID of the Compact in orderData? just need to strip out the most significant 96 bytes and compare the values

[0age]

Ah it looks like we derive it here, makes sense

[snreynolds]

i have a lot of questions as this is my first review on this code base :P will follow up with u @vimageDE

[snreynolds]

whats the reason to enforce sponsors using certain nonces if the nonces are unordered anyways?

in any case it seems like you could use a bitmap here per sponsor so that its not a clean sstore with every single nonce useage if you wanted to optimize?

[mgretzke]

To answer the question about using certain nonces:
The reason a specific nonce is enforced in _checkNonce is to ensure the scope of the nonce matches the sponsor. If a user / sponsor could just use any nonce, it would be possible to block orders / signatures by front-running the nonce.

With this design, the user is able to create and select a random unit96 nonce (still incredibly low chances to hit a nonce previously used by him), without having to call a view function to get the next nonce that is scoped to him.

[mgretzke]

Ah, maybe to provide a crucial piece of information:
The nonce is scoped to the allocator inside of the Compact, not the sponsor. The compact will consume the nonce during the claim process. This ensures replay protection is not dependent on the allocator.

[mgretzke]

Implemented an optimized nonce usage by using a bitmap as suggested!