MultiAggregateRateLimiter - non-evm and multi-lane remote token support

[elatoskinas]

Motivation

Follow up of #916 to integrate non-EVM token support, and convert rateLimitedTokens configuration to be per-chain. This converts the multi-ARL to be truly multi-lane and untied from EVM2EVM to EVM2Any, Any2EVM

Solution

• Add chainSelector to rate limit tokens state
• Convert addresses from bytes to bytes32 (for now assuming that we can fit token addresses in bytes32 for all non-EVM chains)

[github-actions]

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

[github-actions]

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

[github-actions]

LCOV of commit d322f60 during Solidity Foundry #5042

Summary coverage rate:
  lines......: 98.7% (1492 of 1511 lines)
  functions..: 96.5% (301 of 312 functions)
  branches...: 92.8% (646 of 696 branches)

Files changed coverage rate: n/a

[github-actions]

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

[github-actions]

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

• #added For any new functionality added.
• #breaking_change For any functionality that requires manual action for the node to boot.
• #bugfix For bug fixes.
• #changed For any change to the existing functionality.
• #db_update For any feature that introduces updates to database schema.
• #deprecation_notice For any upcoming deprecation functionality.
• #internal For changesets that need to be excluded from the final changelog.
• #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
• #removed For any functionality/config that is removed.
• #updated For any functionality that is updated.
• #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

[connorwstein]

theoretically possible this would be bytes, but we could upgrade worst case

[elatoskinas]

Agreed, we should revisit this when we get to non-EVM, since right now we have not identified addresses with > 32 bytes

[connorwstein]

did we consider deriving these from the router like the token pool does? Simpler config wise and I think it'd also give you the property that the on/offramp can only call their respective methods

[elatoskinas]

I agree with the config simplicity + there would be an added level of misconfiguration since we would check the chain selectors together with the ramps. However, it is a ~10% increase in gas due to the external call:

Function Name	min	avg	median	max	# calls
onIncomingMessage (authorizedCallers)	27390	50521	54146	60411	14
onIncomingMessage (router)	33118	56238	59862	66127	14

I recon the added config redundancy is worth it to save costs in this case, especially since this cost is per-message? Given it is easy to set this config (we can just query the Router ramps in a tooling script and set them to the authorized callers) - should we keep the current approach?

[RensR]

Is it also a 10% increase if the msg came from the router, and therefore the router is hot?

[elatoskinas]

Yeah you're right, I believe it would indeed be the case for the OffRamp, since we check OnlyOffRamp in the same router anyway.

However, the increase could still be there for the OnRamp if there are no token transfers (otherwise there would also be a hot-path through the TokenPool in most cases)

Another aspect is when we have 2 routers at the same time (one test router, and one prod router), it becomes much harder to configure the multi-ARL, since we would typically not re-deploy the multi-ARL. We had this complexity for the Nonce Manager as well, and it looks like the easiest option would be to explicitly maintain the authorized callers set ( I recon we can later refactor out this functionality to an abstract contract that will be re-used by both the multi-ARL and Nonce Manager )

[elatoskinas]

I guess the OnRamp no-tokens case can be optimized - so the gas cost issue can be mitigated:

  function onOutgoingMessage(Client.EVM2AnyMessage memory message, uint64 destChainSelector) external {
    if (message.tokenAmounts.length == 0) {
      return;
    }
    isOnRamp(msg.sender);
  }

I'm still in favour of the current approach though due to the complexity above

[connorwstein]

this would also be the default behaviour right? E.g. say we have a rate limited token, which then is added to a new lane. By default it'd be excluded from the rate limit until we actively enable it with updateRateLimitTokens. Seems potentially safer the other way i.e. default we revert until rate limit set for the new lane

[elatoskinas]

Yes this would be the default behaviour. The change here is mostly a gas optimization - if the rate limiter is disabled, we do not need to loop over all tokens and calculate the total value.

Seems potentially safer the other way i.e. default we revert until rate limit set for the new lane

This would not work because the message validator is called into by all lanes: see OffRamp. If we reverted with an unset rate-limit, we would be forced to set a rate limit config for all lanes. I think it's better to allow the flexibility to partially enable rate limits

[RensR]

Comment on constructor: given this is a chain contract, which will always be deployed before the lanes, I think it's fine to remove the RateLimiterConfigArgs[] memory rateLimiterConfigs args there. I would think it will always be left empty. We could then rm the internal function

[elatoskinas]

True, this is not required now since the contract is completely separate - removing this

[RensR]

Inconsistent order in adds and removed in _applyAuthorizedCallerUpdates. We usually remove first, then add. Both in args and in logic