Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxLogsKept = 1 for contract transmitter #1126

Draft
wants to merge 994 commits into
base: release/2.12.0-ccip1.4
Choose a base branch
from

Conversation

mateusz-sekara
Copy link
Contributor

Motivation

Solution

Copy link
Contributor

github-actions bot commented Jul 2, 2024

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.

0xsuryansh and others added 6 commits August 8, 2024 23:19
## 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 <[email protected]>
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>
## Motivation
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



## Solution

- 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 <[email protected]>
Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: 0xsuryansh <[email protected]>
Co-authored-by: Rens Rooimans <[email protected]>
Final release of all 1.5 contracts - don't merge before the other PRs
are all in


TODO
- #1258
- #1256
- #1273

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
## Motivation
Static price removal job rollout will be delayed to after 1.5 release.
To unblock db load concerns in 1.4.21 which writes prices to db, we want
to reduce number of token-price related insertions in db.

## Solution
Separate gas price and token price insertion frequency, insert every 10
minutes for token price. 10-min resolution for token price is accurate
enough for our use case.
anirudhwarrier and others added 5 commits August 12, 2024 12:59
## Problem
The previous shared wallet used in the live network tests is
discontinued and we are advised to use new
wallet(`0xB509c046e1182c7B36d2D9733554BC268716803C`) provided by
Security team and the private key of this wallet is stored in github
secret name `QA_SHARED_803C_KEY`. Need to wire this secret to our test
config to consume the private key in our test.

https://smartcontract-it.atlassian.net/browse/CCIP-2875

## Solution

1. Added a stop gap solution until
[testsecrets](#1189)
changes are merged and secrets are modified.
2. Enable the workflow schedule

---------

Co-authored-by: Balamurali Gopalswami <[email protected]>
Co-authored-by: Balamurali Gopalswami <[email protected]>
## Motivation
Price reporting frequencies have been turned to following values on Eth
mainnet (where most price update costs is)
```
GasHeartBeat: 2hr
ExecGasPriceDeviation: 400%
DAPriceDeviation: 400%
TokenPriceHeartBeat: 12hr
TokenPriceDeviation: 20%
```

As a result, updates are mostly heartbeat driven as opposed to deviation
driven. A simple trick to improve leader-lane batching is to always
batch report all heartbeat prices.

In clam environments where prices are heartbeat driven, this absolutely
minimizes number of price reports.

In flux environments where good number of prices are deviation driven
while some remain heartbeat driven, e.g. in case of Eth blob base fee
spike, the penalty we take for including each additional prices is low
(~6k gas) v.s the OCR tx overhead (~110k gas)
## Motivation


## Solution
## Motivation

Recent merges to chainlink-ccip have caused the integration test to
break.

## Solution

Include the gas estimator changes required as well as some token data
reader changes.

Include fixes from
smartcontractkit/chainlink-ccip#60
## 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>
b-gopalswami and others added 4 commits August 13, 2024 07:48
## Motivation
I guess this mis-aligned comment statement is a problem in a yml file.
Hope this resolves the failure mentioned
[here](https://github.com/smartcontractkit/ccip/actions/runs/10360863386/workflow#L143)

## Solution
Align and try.
…instead of bytes32 (#1207)

## 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

```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;
```

## Change List:

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>
## Motivation

Bump chainlink-ccip to the version from this PR:
smartcontractkit/chainlink-ccip#64

## Solution

Bump and fix the tests.
## Motivation

Routine bump of chainlink-ccip after the
smartcontractkit/chainlink-ccip#64 PR was
merged. This has no functional changes, see
#1291 for the PR that fixed
the integration tests.

## Solution

Bump chainlink-ccip.
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.