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

CCIP gethwrappers with zkstack support (integrated with main flow) #15487

Open
wants to merge 50 commits into
base: develop
Choose a base branch
from

Conversation

yashnevatia
Copy link

@yashnevatia yashnevatia commented Dec 3, 2024

This breaks integration-tests because:

  • Adding geth wrappers with zksync logic
  • This enables us to use the same wrappers for ethereum and zksync chains
    • It prevents adding if conditions to every deploy method we have across the different tools
  • The existing deployment tools will only have to change the way they instantiate the TransactOps that goes into the deploy method, but from our understanding thats only one once per tool (change given below)

Instead of doing this

auth, err := bind.NewKeyedTransactorWithChainID(ownerKey, big.NewInt(int64(chainID)))

We will now do this, where we add a zksync wallet into the context.value of the TransactOps

auth, err := bind.NewKeyedTransactorWithChainID(ownerKey, big.NewInt(int64(chainID)))
if generated_zks.IsZKSync(client) {
  zkclient := zkSyncClient.NewClient(client.Client())
  wallet, err := zkSyncAccounts.NewWallet(common.Hex2Bytes(privateKeyHex), zkclient, nil)
  ctx := context.WithValue(auth.Context, walletKey, wallet)
  auth.Context = ctx
}

If the above is done, we can simply do the below for deploying to both ethereum and zkstack chains (no special logic for zkstack)

linkTokenAddress, deployTx, token, err := link_token.DeployLinkToken(auth, client)

TODO:

  • WETH and ICCIPEncodingUtils are not compiling -> need to work on that
  • make the class name and type names in abigen.go constants

WETH9.sol compilation breaks

--> /Users/yashvardhan/chainlink/contracts/src/v0.8/ccip/test/WETH9.sol

Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
--> /Users/yashvardhan/chainlink/contracts/src/v0.8/ccip/test/WETH9.sol


Error: You are using '<address payable>.send/transfer(<X>)' without providing the gas amount.
Such calls will fail depending on the pubdata costs.

Requires

Supports

@yashnevatia yashnevatia requested review from a team as code owners December 3, 2024 11:31
@yashnevatia yashnevatia changed the title Zksync wrappers integrated CCIP gethwrappers with zkstack support (integrated with main flow) Dec 3, 2024
@yashnevatia yashnevatia requested a review from a team as a code owner December 4, 2024 09:47
@@ -38,6 +39,13 @@ compileContract () {
--bin-runtime --hashes --metadata --metadata-literal --combined-json abi,hashes,metadata,srcmap,srcmap-runtime \
--evm-version paris \
"$ROOT"/contracts/src/v0.8/"$1"

zksolc --overwrite -O3 --metadata-hash none \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the overwrite same as the "mode"?
& any other option to provide "fallback_to_optimizing_for_size"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--fallback-Oz                 Try to recompile with -Oz if the bytecode is too large

i think unless you dont specify this flag, it should be false.
--overwrite is for overwriting a file if the wrapper already exists

@@ -78,11 +86,11 @@ compileContract ccip/test/helpers/receivers/MaybeRevertMessageReceiver.sol
compileContract ccip/test/helpers/MultiOCR3Helper.sol
compileContract ccip/test/mocks/MockE2EUSDCTokenMessenger.sol
compileContract ccip/test/mocks/MockE2EUSDCTransmitter.sol
compileContract ccip/test/WETH9.sol
# compileContract ccip/test/WETH9.sol (error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can completely exclude these test contracts? no?

compileContract ccip/test/helpers/CCIPReaderTester.sol

# Encoding Utils
compileContract ccip/interfaces/encodingutils/ICCIPEncodingUtils.sol
# compileContract ccip/interfaces/encodingutils/ICCIPEncodingUtils.sol (error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, what is the error here? something we can fix? or is this file not being used by any contract?

Copy link
Author

@yashnevatia yashnevatia Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It compiles

Compiler run successful. Artifact(s) can be found in directory "/Users/yashvardhan/chainlink/contracts/zksolc/v1.5.3/ICCIPEncodingUtils".

but does not generate the ICCIPEncodingUtils binary
image

set -e

# Specify the version of zksolc you want to install
VERSION="1.5.6"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about parameterising this for both the Zksolc setup and the Solidity compilation script so that the products and the user can choose the version of the contracts to be compiled? for example ccip 1.5.3, automation could be a different version, even within ccip, MCMS contracts have a different version

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

github-actions bot commented Dec 4, 2024

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@app-token-issuer-infra-releng app-token-issuer-infra-releng bot requested a review from a team as a code owner December 6, 2024 08:03
Copy link
Contributor

github-actions bot commented Dec 6, 2024

It appears that the changeset file you've added or modified in contracts/.changeset is not valid.
Please delete the file and run pnpm changeset in the contracts directory.

Copy link
Contributor

github-actions bot commented Dec 6, 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.

🎖️ No JIRA issue number found in: PR title, commit message, or branch name. Please include the issue ID in one of these.

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.

2 participants