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

Fixes to capabilities and contracts merges [CCIP-2946] #14266

Merged
merged 31 commits into from
Aug 28, 2024

Conversation

asoliman92
Copy link
Contributor

@asoliman92 asoliman92 commented Aug 28, 2024

Relates to #14260

This fixes all conflicts and makes sure that contracts and gethwrappers are identical to the ones on CCIP.

To review this PR you need the following:

  1. Checkout the branch locally
  2. Run the given script (make sure both chainlink and ccip repos have same parent directory and that the script is with them in the parent)

./check-diff.sh contracts/src/v0.8/ccip
expected output:

Source files not modified:
     174
Source files modified:

Then
./check-diff.sh core/gethwrappers/ccip
expected ouptut:

ccip repo commit: 
8d54185c8f (HEAD -> ccip-develop, origin/ccip-develop, origin/HEAD) Multi-Mechanism USDC Token Pools (#1280)
chainlink repo commit: 
7acdd14a1b (HEAD -> ccip/capabilities-debug-fixes, origin/ccip/capabilities-debug-fixes) Remove ccip from integration-tests/deployment

Source files not modified:
      64
Source files modified:

Script:

#!/usr/bin/env bash

# Check if the directory is passed as an argument
if [ -z "$1" ]; then
    echo "Usage: $0 <directory>"
    exit 1
fi

# Directory to check for parity
CHECK_DIR="$1"

if [[ "$OSTYPE" == "darwin"* ]]; then
    # macOS
    SUM=gmd5sum
else
    # Assume Linux
    SUM=md5sum
fi

pushd ccip/$CHECK_DIR > /dev/null
echo -e "ccip repo commit: \n$(git log --max-count=1 --oneline --decorate)"
find . -type f -exec $SUM {} \;  > ../files.md5
popd > /dev/null
mv ccip/$CHECK_DIR/../files.md5 ./chainlink/$CHECK_DIR/../files.md5
pushd ./chainlink/$CHECK_DIR > /dev/null
echo -e "chainlink repo commit: \n$(git log --max-count=1 --oneline --decorate)"

echo ""

echo "Source files not modified:"
$SUM -c ../files.md5 2>/dev/null | grep OK | wc -l
echo "Source files modified:"
$SUM -c ../files.md5 2>/dev/null | grep -v OK

For this reason both forge fmt and solidity lint are failing the CI checks. We're taking identical copy and we're not interested in formatting them for now to make it easier reviewing the branch.

Once both are good to go please review the offchain changes and any other changes.

@asoliman92 asoliman92 changed the title Fixes to capabilities and contracts merges Fixes to capabilities and contracts merges [CCIP-2946] Aug 28, 2024
@winder
Copy link
Contributor

winder commented Aug 28, 2024

Changes in this PR are almost identical to the ccip files:

./check-diff.sh contracts/src/v0.8/ccip

ccip repo commit:
8d54185c8f (HEAD) Multi-Mechanism USDC Token Pools (#1280)
chainlink repo commit:
d0d3e58a60 (HEAD -> ccip/capabilities-debug-fixes, origin/ccip/capabilities-debug-fixes) Revert contract auto linting

Source files not modified:
173
Source files modified:
./test/NonceManager.t.sol: FAILED
./check-diff.sh core/gethwrappers/ccip

ccip repo commit:
8d54185c8f (HEAD) Multi-Mechanism USDC Token Pools (#1280)
chainlink repo commit:
d0d3e58a60 (HEAD -> ccip/capabilities-debug-fixes, origin/ccip/capabilities-debug-fixes) Revert contract auto linting

Source files not modified:
63
Source files modified:
./generated/price_registry_1_2_0/price_registry.go: FAILED

There seem to be some formatting differences on these two files.

@asoliman92 asoliman92 force-pushed the ccip/capabilities-debug-fixes branch from d0d3e58 to 75aa738 Compare August 28, 2024 19:07
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
5.5% Duplication on New Code

See analysis details on SonarQube

Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Ran again with the latest commit and these directories are now identical.

I'm not sure how to review the other changes exactly. Since they're being merged into the feature branch, I think they can be reviewed in the next PR.

@asoliman92 asoliman92 marked this pull request as ready for review August 28, 2024 19:49
@asoliman92 asoliman92 requested review from reductionista and removed request for a team August 28, 2024 19:49
@asoliman92 asoliman92 merged commit c323e0d into ccip/capabilities-debug Aug 28, 2024
164 of 168 checks passed
@asoliman92 asoliman92 deleted the ccip/capabilities-debug-fixes branch August 28, 2024 19:50
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