feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 #1207

defistar · 2024-07-17T18:43:10Z

Description:

rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32

Details

In the MultiAggregateRateLimiter contract, the the following mapping might be simplifiable to
(uint64 remoteChainSelector -> address token) ,
since it is only used as an isEnabled check (i.e. the (address token -> bytes32 remoteToken) is never used on-chain)
mapping being necessary off-chain, we need to convert the (address -> bytes32) mapping to (address -> bytes) to be consistent with using bytes for all family-agnostic addresses

mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytes32Map tokensLocalToRemote) internal
    s_rateLimitedTokensLocalToRemote;

this has to been changed to

  mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytesMap tokensLocalToRemote) internal
    s_rateLimitedTokensLocalToRemote;

Change List:

New solidity library EnumerableMapBytes32 which contains Bytes32ToBytes Mapping and enumerate it
EnumerableMapAddresses.sol library has added support for the new library cherrypicked from chainlink repo EnumerableMapBytes32
MultiAggregateRateLimiter with mapping for remoteSelector -> localTokenAddress -> remoteTokenAddress is updated to contain remoteTokenAddress as bytes instead of bytes32

…ted to take bytes instead of bytes32

github-actions · 2024-07-17T18:52:29Z

LCOV of commit `f9586b3` during Solidity Foundry #7226

Summary coverage rate:
  lines......: 98.7% (1882 of 1907 lines)
  functions..: 96.4% (347 of 360 functions)
  branches...: 90.6% (797 of 880 branches)

Files changed coverage rate: n/a

…d its internal function usage

elatoskinas · 2024-07-18T10:26:25Z

contracts/src/v0.8/ccip/test/rateLimiter/MultiAggregateRateLimiter.t.sol

@@ -482,14 +482,14 @@ contract MultiAggregateRateLimiter_updateRateLimitTokens is MultiAggregateRateLi
        remoteChainSelector: CHAIN_SELECTOR_1,
        localToken: s_destTokens[0]
      }),
-      remoteToken: bytes32(bytes20(s_sourceTokens[0]))
+      remoteToken: abi.encodePacked(s_sourceTokens[0])


issue: we use abi.encode for addresses -> bytes conversion instead of encodePacked - e.g. see https://github.com/smartcontractkit/ccip/blob/ccip-develop/contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol#L371

contracts/src/v0.8/shared/enumerable/CCIPEnumerableMap.sol

contracts/src/v0.8/shared/enumerable/EnumerableMapAddresses.sol

contracts/src/v0.8/ccip/MultiAggregateRateLimiter.sol

… mapping

…apping

elatoskinas · 2024-08-13T08:38:32Z

contracts/src/v0.8/shared/enumerable/EnumerableMapAddresses.sol

@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: MIT


We should cherry-pick the commit from chainlink to make this lib in sync:

Add chainlink remote via: git remote add

git cherry-pick <commit-id>

This looks like it's missing the src/v0.8/shared/test/enumerable files

contracts/src/v0.8/ccip/MultiAggregateRateLimiter.sol

contracts/src/v0.8/ccip/test/pools/LockReleaseTokenPool.t.sol

contracts/src/v0.8/ccip/test/rateLimiter/MultiAggregateRateLimiter.t.sol

elatoskinas · 2024-08-13T08:41:42Z

contracts/src/v0.8/ccip/test/rateLimiter/MultiAggregateRateLimiter.t.sol

@@ -918,7 +915,7 @@ contract MultiAggregateRateLimiter_onOutboundMessage is MultiAggregateRateLimite
          remoteChainSelector: CHAIN_SELECTOR_1,
          localToken: s_sourceTokens[i]
        }),
-        remoteToken: bytes32(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))
+        remoteToken: abi.encodePacked(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))


Suggested change

remoteToken: abi.encodePacked(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))

remoteToken: abi.encode(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))

contracts/src/v0.8/ccip/test/rateLimiter/MultiAggregateRateLimiter.t.sol

…apping

* feat: CCIP-2465 enumerableMap for addressToBytes Mapping * fix: CCIP-2465 prettier write for new library contracts * fix: CCIP-2465 gas-snapshot updated and changeset added for new library * fix: CCIP-2465 gassnapshot corrections for EnumerableAddresses

…teLimiter

cl-sonarqube-production · 2024-08-14T04:53:30Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

RayXpub · 2024-08-14T06:30:05Z

contracts/src/v0.8/shared/enumerable/EnumerableMapAddresses.sol

 import {EnumerableMap} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableMap.sol";
+import {EnumerableMapBytes32} from "./EnumerableMapBytes32.sol";


nit: probably not for this PR but now that we have OZ v5 we might want to update this

got it will add a note for this one

…instead of bytes32 (#1207) rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 - In the MultiAggregateRateLimiter contract, the the following mapping might be simplifiable to - (uint64 remoteChainSelector -> address token) , - since it is only used as an isEnabled check (i.e. the (address token -> bytes32 remoteToken) is never used on-chain) - mapping being necessary off-chain, we need to convert the (address -> bytes32) mapping to (address -> bytes) to be consistent with using bytes for all family-agnostic addresses ```js mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytes32Map tokensLocalToRemote) internal s_rateLimitedTokensLocalToRemote; ``` this has to been changed to ```js mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytesMap tokensLocalToRemote) internal s_rateLimitedTokensLocalToRemote; ``` 1. New solidity library `EnumerableMapBytes32` which contains `Bytes32ToBytes` Mapping and enumerate it 2. `EnumerableMapAddresses.sol` library has added support for the new library cherrypicked from chainlink repo `EnumerableMapBytes32` 3. `MultiAggregateRateLimiter` with mapping for remoteSelector -> localTokenAddress -> remoteTokenAddress is updated to contain remoteTokenAddress as `bytes` instead of `bytes32` --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>

feat: CCIP-2465 rateLimiterConfig mapping of localToremoteTokens upda…

8b03093

…ted to take bytes instead of bytes32

defistar temporarily deployed to sdlc July 17, 2024 18:43 — with GitHub Actions Inactive

defistar changed the title ~~feat: CCIP-2465 rateLimiterConfig mapping of localToremoteTokens upda…~~ feat: rateLimiterConfig mapping of localToremoteTokens to take bytes instead of bytes32 Jul 17, 2024

fix: CCIP-2465 gassnapshot updated

aa6bec1

defistar temporarily deployed to sdlc July 17, 2024 19:01 — with GitHub Actions Inactive

defistar requested review from RensR, elatoskinas and 0xsuryansh July 17, 2024 19:03

defistar self-assigned this Jul 17, 2024

defistar added the looking for review This needs more reviewers label Jul 17, 2024

fix: CCIP-2465 formatting and lint fixes for EnumerableMap Library an…

fcc0c9f

…d its internal function usage

defistar temporarily deployed to sdlc July 17, 2024 19:32 — with GitHub Actions Inactive

defistar marked this pull request as ready for review July 17, 2024 19:33

defistar requested review from makramkd, RayXpub and a team as code owners July 17, 2024 19:34

fix: CCIP-2465 prettier formatting for EnumerableMap Library contracts

b62672e

defistar temporarily deployed to sdlc July 18, 2024 02:35 — with GitHub Actions Inactive

defistar changed the title ~~feat: rateLimiterConfig mapping of localToremoteTokens to take bytes instead of bytes32~~ feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 Jul 18, 2024

elatoskinas reviewed Jul 18, 2024

View reviewed changes

defistar and others added 4 commits July 19, 2024 08:46

fix: CCIP-2465 fix EnumerableMap references compilation fixes

83f9b2f

feat: CCIP-2465 use EnumerableMapBytes32 library for address -> bytes…

bd1e9a6

… mapping

Merge branch 'ccip-develop' into feature/CCIP-2465-rateLimiter-tokenM…

a7a02c6

…apping

Merge a7a02c6 into a4b734a

231c1d9

defistar temporarily deployed to sdlc August 12, 2024 14:32 — with GitHub Actions Inactive

Update gethwrappers

2940330

app-token-issuer-infra-releng bot temporarily deployed to sdlc August 12, 2024 14:39 Inactive

fix: CCIP-2465 gassnapshots updated

a2f7724

fix: CCIP-2465 MultiAggregatorRateLimiter test fixes

41a1795

defistar temporarily deployed to sdlc August 12, 2024 19:07 — with GitHub Actions Inactive

defistar requested a review from elatoskinas August 12, 2024 19:08

elatoskinas reviewed Aug 13, 2024

View reviewed changes

Merge branch 'ccip-develop' into feature/CCIP-2465-rateLimiter-tokenM…

b62788e

…apping

defistar temporarily deployed to sdlc August 13, 2024 18:57 — with GitHub Actions Inactive

fix: CCIP-2465 LockReleaseTokenPool test fixes

1ddb158

defistar temporarily deployed to sdlc August 13, 2024 21:30 — with GitHub Actions Inactive

defistar requested a review from a team as a code owner August 14, 2024 03:45

defistar had a problem deploying to sdlc August 14, 2024 03:46 — with GitHub Actions Failure

defistar force-pushed the feature/CCIP-2465-rateLimiter-tokenMapping branch from 8d916f8 to 1ddb158 Compare August 14, 2024 03:49

defistar temporarily deployed to sdlc August 14, 2024 03:49 — with GitHub Actions Inactive

fix: CCIP-2465 refactor MultiAggregatorRateLimiter test

93d935a

defistar temporarily deployed to sdlc August 14, 2024 03:54 — with GitHub Actions Inactive

defistar and others added 2 commits August 14, 2024 09:38

fix: CCIP-2465 forge formatting fix for RateLimiter test

cc00380

defistar temporarily deployed to sdlc August 14, 2024 04:09 — with GitHub Actions Inactive

fix: CCIP-2465 remove obselete library references in MultiAggregateRa…

902359b

…teLimiter

defistar temporarily deployed to sdlc August 14, 2024 04:24 — with GitHub Actions Inactive

fix: CCIP-2465 gassnapshots fixes

f9586b3

defistar temporarily deployed to sdlc August 14, 2024 04:33 — with GitHub Actions Inactive

defistar requested a review from elatoskinas August 14, 2024 04:33

0xsuryansh approved these changes Aug 14, 2024

View reviewed changes

RayXpub reviewed Aug 14, 2024

View reviewed changes

RayXpub approved these changes Aug 14, 2024

View reviewed changes

elatoskinas approved these changes Aug 14, 2024

View reviewed changes

defistar merged commit deec42f into ccip-develop Aug 14, 2024
109 checks passed

defistar deleted the feature/CCIP-2465-rateLimiter-tokenMapping branch August 14, 2024 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 #1207

feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 #1207

defistar commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

elatoskinas Jul 18, 2024

defistar Aug 12, 2024

elatoskinas Aug 13, 2024

defistar Aug 14, 2024

elatoskinas Aug 13, 2024

cl-sonarqube-production bot commented Aug 14, 2024

RayXpub Aug 14, 2024

defistar Aug 14, 2024

	remoteToken: abi.encodePacked(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))
	remoteToken: abi.encode(bytes20(s_destTokenBySourceToken[s_sourceTokens[i]]))

		import {EnumerableMap} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableMap.sol";
		import {EnumerableMapBytes32} from "./EnumerableMapBytes32.sol";

feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 #1207

feat: rateLimiterConfig mapping of localToRemoteTokens to take bytes instead of bytes32 #1207

Conversation

defistar commented Jul 17, 2024 • edited Loading

Description:

Details

Change List:

github-actions bot commented Jul 17, 2024 • edited Loading

LCOV of commit f9586b3 during Solidity Foundry #7226

elatoskinas Jul 18, 2024

Choose a reason for hiding this comment

defistar Aug 12, 2024

Choose a reason for hiding this comment

elatoskinas Aug 13, 2024

Choose a reason for hiding this comment

defistar Aug 14, 2024

Choose a reason for hiding this comment

elatoskinas Aug 13, 2024

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Aug 14, 2024

Quality Gate passed

RayXpub Aug 14, 2024

Choose a reason for hiding this comment

defistar Aug 14, 2024

Choose a reason for hiding this comment

defistar commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

LCOV of commit `f9586b3` during Solidity Foundry #7226