Merge CCIP capabilities latest [CCIP-2946] #14242
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Various minor onchain fixes - improve accounting for token payload data - rm old check in usdc pool - fix comment on precompile range - add transferLiquidity function to LockReleaseTokenPool - move rate limit admin logic from lockRelease pool to TokenPool New changes - removed defaultTokenDestBytesOverhead and used CCIP_LOCK_OR_BURN_V1_RET_BYTES as default value - Add uint32 destGasAmount to the SourceTokenData struct to allow us to send the amount we billed on source to dest - This single value is used for the releaseOrMint and what is left after that call is used for transfer - Removed the defaults for releaseOrMint and transfer from the offRamp --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
## Motivation `NonceManager` and `MultiAggregateRateLimiter` contracts were missing `typeAndVersion`. ## Solution Add `ITypeAndVersion` inheritance to both contracts.
Since the separation of manual exec window and DON exec window, it would theoretically be possible that two executions happen at the same time (although unlikely). Remove reverts to ensure other msgs in the batches are unaffected --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
## Motivation This PR applies several fixes from static analysis findings. ## Solution Fixes include: - Unused state variables removal - Unused imports removal - Unused events removal - Unused custom errors removal - Add of missing zero address check - Implementation of missing getter - Use of constants instead of magic numbers
## Motivation We want the ability to test new lanes using custom, test routers. Therefore, we need a one-to-many relationship b/t routers and lanes, rather than a singleton. ## Solution Make routers configurable per lane, rather than per contract
## Motivation Change the hop through the offRamp to a transferFrom so the funds never touch CCIP contracts outside of the pool ## Solution
## Motivation Change `typeAndVersion`of RMN contract from `RMN 1.5.0-dev` to `RMN 1.5.0` since the contract is finalized. --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Rens Rooimans <github@rensrooimans.nl>
## Motivation `EVM2EVMMultiOffRamp.executeSingleMessage` is optimised by replacing memory to calldata, internal functions `_releaseOrMintSingleTokens` and `_releaseOrMintSingleToken` can be optimised ``` function executeSingleMessage(Internal.Any2EVMRampMessage memory message, bytes[] memory offchainTokenData) external ``` Tradeoff is increased contract size, which is exceeding the limit and thus we have to decrease the optimiser runs ## Findings After replacing memory with call data we have the following findings for gas and contract size | Test | Before | after | delta | | ------------------------------------------------------------------------------------------- | ------ | ------ | ------ | | EVM2EVMMultiOffRamp_executeSingleMessage:test_NonContractWithTokens_Success() | 249368 | 247671 | \-1697 | | EVM2EVMMultiOffRamp_executeSingleMessage:test_NonContract_Success() | 20672 | 19245 | \-1427 | | EVM2EVMMultiOffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens_Success() | 48381 | 47265 | \-1116 | | EVM2EVMMultiOffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens_Success() | 278146 | 276759 | \-1387 | | EVM2EVMMultiOffRamp_executeSingleMessage:test_executeSingleMessage_WithValidation_Success() | 93615 | 92499 | \-1116 | | EVM2EVMMultiOffRamp__releaseOrMintSingleToken:test__releaseOrMintSingleToken_Success() | 108343 | 107939 | \-404 | | **Optimser Run** | **Size** | **Margin(kB)** | **With call data optimisation** | | ---------------- | --------------------------------- | -------------- | ------------------------------- | | 2500 | 24.113 | 0.463 | No | | 2500 | 24.857 (size increase of 0.744kB) | \-0.281 | Yes | | 2400 | 24.635 | \-0.059 | Yes | | 2200 | 24.635 | \-0.059 | | | 2000 | 24.572 | 0.004 | Yes | --------- Signed-off-by: 0xsuryansh <suryansh.shrivastava@smartcontract.com> Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
## Motivation It is significantly cheaper to assert balances than do the transferFrom ## Solution Undo recent approve changes, add balance assertions had to undo some of the multiOfframp calldata changes to make it fit --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
currently, gas amounts are placed in the SourceTokenData. However, for
the multi-ramps, the token data should be chain-family agnostic, which
will likely require the destGasAmount lift to another field
- add a new Struct which holds the receiverExecutionGasLimit and
transferGasAmounts
```js
struct GasLimitOverride {
uint256 receiverExecutionGasLimit;
uint256[] tokenGasOverrides;
}
```
- `tokenGasOverrides` is an array of GasLimits to be used during the
`relaseOrMint` call for the specific tokenPool associated with token
---------
Signed-off-by: 0xsuryansh <suryansh.shrivastava@smartcontract.com>
Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: 0xsuryansh <suryansh.shrivastava@smartcontract.com>
Co-authored-by: Rens Rooimans <github@rensrooimans.nl>
Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
## Motivation ## Solution
## Motivation Use the latest version of deps ## Solution Upgrade OZ deps to latest version ## Implementation Notes `IERC20` and `SafeERC20` are kept on `4.8.3` because of the `^0.8.20` requirement OZ imposes on the v5 contracts. We can't upgrade these deps until the rest of the contract codebase upgrades to 0.8.20+ --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
…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>
- Fix stale comment - remove wrapper gen for the old mockRMN - should no longer be used in offchain tests, use the real RMN instead - Remove naming clash with RMN
There's already 1.4 pools out there of this type so we need a proxy
* setter of a bool representing the `allowOutOfOrderExecution` * creating and passing `extraArgs` to the message
## Motivation getPreviousPool was missing ## Solution Add getPreviousPool
CCIP Config can go to larger size and any query from offchain components via rpc call can cause timeout issues add pagination to `getAllCCIPConfig` function which takes - pageSize - startIndex --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Makram Kamaleddine <makramkd@users.noreply.github.com>
#1310) ## Motivation gasUsed for Execution to be emitted along with ExecutionStateChangedEvent ## Solution compute `gasUsed` for execution of a message in EVM2EVMMultiOffRamp this change is applicable to only 1.6 version Test Assertion must be added to assert the event body parameters (excluding the gasUsed as it cant be hardcoded in tests) ** This is extension of the closed PR: smartcontractkit/ccip#1297 got signature verification issue with other PR. so moving all changes over here --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> Co-authored-by: Ryan <80392855+RayXpub@users.noreply.github.com>
Cleanup & more realistic values for gas overheads
Depends on : #13878 The price registry needs to conform to the KeystoneFeedsConsumer interface in order to receive keystone price feed updates. Implemented Keystones `IReciever` interface `onReport` function to handle the following report type ``` struct ReceivedFeedReport { address Token; uint224 Price; uint32 Timestamp; } ``` and storing the reported fee in `Internal.TimestampedPackedUint224` for `s_usdPerToken` mapping --------- Signed-off-by: 0xsuryansh <suryansh.shrivastava@smartcontract.com>
## Motivation fix: CCIP-3095 fix flaky test assertions in offramp tests Few unit tests were still asserting on `gasUsed` in `ExecutionStateChanged` event emitted by offramp contract while executing messages in the report. ## Solution use assertion helper function to assert the ExecutionStateChanged Event indexed topics and chosen fields of event data
## Motivation The goal of this PR is to cleanup comments. ## Solution Update/remove stale comments, fix typos and standardize comments style. Additional fixes: - Consistent use of named return variables - ~~Reordering of add/remove operations (add + remove -> remove + add) for the following functions in the `PriceRegistry`:~~ - ~~`applyTokenTransferFeeConfigUpdates`~~ - ~~`applyFeeTokensUpdates`~~ --------- Co-authored-by: Evaldas Latoškinas <34982762+elatoskinas@users.noreply.github.com>
Co-authored-by: Ryan Hall <hall.ryan.r@gmail.com>
Static analysis results are availableHey @asoliman92, you can view Slither reports in the job summary here or download them as artifact here. Please check them before merging and make sure you have addressed all issues. |
Rename the PriceRegistry to FeeQuoter. This PR restores the 1.2 PriceRegistry interface which is still used for CCIP 1.5. The FeeQuoter is a superset of that interface, which is why all tests can use the feeQuoter, even in 1.5. This PR does NOT claim to rename all offchain instances. It only handles onchain and wrapper/mock based references
…link into ccip/capabilities
asoliman92
changed the title
This reverts commit d6e0b04.
Error: integration-tests/actions/actions.go:493:88: not enough arguments in call to seth.Decode have (unknown type) want (*"github.com/ethereum/go-ethereum/core/types".Transaction, error) (typecheck) decodedTx, err := seth.Decode(operatorFactoryInstance.DeployNewOperatorAndForwarder())
asoliman92
force-pushed
the
ccip/capabilities
branch
from
fb2bee6 to
7341b06
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.