-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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-4948: Adding new test for disable and enable lane #16038
Changes from 9 commits
83533b9
48c7cb7
bbc49ec
0824a15
093222d
ed042b6
2a3a956
0ad73f4
2dbae80
d957762
769e9b2
193de02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -482,6 +482,65 @@ func AddLane( | |
require.NoError(t, err) | ||
} | ||
|
||
// RemoveLane removes a lane between the source and destination chains in the deployed environment. | ||
func RemoveLane(t *testing.T, e *DeployedEnv, src, dest uint64, isTestRouter bool) { | ||
var err error | ||
apps := []commoncs.ChangesetApplication{ | ||
{ | ||
Changeset: commoncs.WrapChangeSet(changeset.UpdateRouterRampsChangeset), | ||
Config: changeset.UpdateRouterRampsConfig{ | ||
UpdatesByChain: map[uint64]changeset.RouterUpdates{ | ||
// onRamp update on source chain | ||
src: { | ||
OnRampUpdates: map[uint64]bool{ | ||
dest: false, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
{ | ||
Changeset: commoncs.WrapChangeSet(changeset.UpdateFeeQuoterDestsChangeset), | ||
Config: changeset.UpdateFeeQuoterDestsConfig{ | ||
UpdatesByChain: map[uint64]map[uint64]fee_quoter.FeeQuoterDestChainConfig{ | ||
src: { | ||
dest: changeset.DefaultFeeQuoterDestChainConfig(false), | ||
}, | ||
}, | ||
}, | ||
}, | ||
{ | ||
Changeset: commoncs.WrapChangeSet(changeset.UpdateOnRampsDestsChangeset), | ||
Config: changeset.UpdateOnRampDestsConfig{ | ||
UpdatesByChain: map[uint64]map[uint64]changeset.OnRampDestinationUpdate{ | ||
src: { | ||
dest: { | ||
IsEnabled: false, | ||
TestRouter: isTestRouter, | ||
AllowListEnabled: false, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
{ | ||
Changeset: commoncs.WrapChangeSet(changeset.UpdateOffRampSourcesChangeset), | ||
Config: changeset.UpdateOffRampSourcesConfig{ | ||
UpdatesByChain: map[uint64]map[uint64]changeset.OffRampSourceUpdate{ | ||
dest: { | ||
src: { | ||
IsEnabled: false, | ||
TestRouter: isTestRouter, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @makramkd would it affect the inflight requests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, no offramp changes should be made There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my observation, if no offramp changes are made, then in-flight requests are getting delivered. Let's say, we made the offramp changes as well, in that case the in-flight requests are not getting delivered. Haven't seen reverting as well not sure If I need to wait longer for that. So, we should not make offramp changes when there is in-flight request. Will remove this part. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, just to be clear this is the desired behavior. |
||
e.Env, err = commoncs.ApplyChangesets(t, e.Env, e.TimelockContracts(t), apps) | ||
require.NoError(t, err) | ||
} | ||
|
||
func AddLaneWithDefaultPricesAndFeeQuoterConfig(t *testing.T, e *DeployedEnv, state changeset.CCIPOnChainState, from, to uint64, isTestRouter bool) { | ||
stateChainFrom := state.Chains[from] | ||
AddLane( | ||
|
@@ -494,7 +553,7 @@ func AddLaneWithDefaultPricesAndFeeQuoterConfig(t *testing.T, e *DeployedEnv, st | |
}, map[common.Address]*big.Int{ | ||
stateChainFrom.LinkToken.Address(): DefaultLinkPrice, | ||
stateChainFrom.Weth9.Address(): DefaultWethPrice, | ||
}, changeset.DefaultFeeQuoterDestChainConfig()) | ||
}, changeset.DefaultFeeQuoterDestChainConfig(true)) | ||
} | ||
|
||
// AddLanesForAll adds densely connected lanes for all chains in the environment so that each chain | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
package ccip | ||
|
||
import ( | ||
"math/big" | ||
"testing" | ||
|
||
"github.com/ethereum/go-ethereum/accounts/abi/bind" | ||
"github.com/ethereum/go-ethereum/common" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/smartcontractkit/chainlink-common/pkg/utils/tests" | ||
"github.com/smartcontractkit/chainlink-testing-framework/lib/utils/testcontext" | ||
"github.com/smartcontractkit/chainlink/deployment" | ||
"github.com/smartcontractkit/chainlink/deployment/ccip/changeset" | ||
"github.com/smartcontractkit/chainlink/deployment/ccip/changeset/testhelpers" | ||
testsetups "github.com/smartcontractkit/chainlink/integration-tests/testsetups/ccip" | ||
"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/ccip/generated/onramp" | ||
"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/ccip/generated/router" | ||
) | ||
|
||
// Intention of this test is to ensure that the lane can be disabled and enabled correctly without disrupting the other lanes. | ||
func TestDisableLane(t *testing.T) { | ||
tenv, _, _ := testsetups.NewIntegrationEnvironment(t, | ||
testhelpers.WithNumOfChains(3), | ||
testhelpers.WithNumOfUsersPerChain(2), | ||
) | ||
|
||
e := tenv.Env | ||
state, err := changeset.LoadOnchainState(e) | ||
require.NoError(t, err) | ||
|
||
// add all lanes | ||
testhelpers.AddLanesForAll(t, &tenv, state) | ||
|
||
var ( | ||
chains = e.AllChainSelectors() | ||
chainA, chainB, chainC = chains[0], chains[1], chains[2] | ||
expectedSeqNumExec = make(map[testhelpers.SourceDestPair][]uint64) | ||
startBlocks = make(map[uint64]*uint64) | ||
pairs = testsetups.GetSourceDestPairs([]uint64{chainA, chainB, chainC}) | ||
|
||
sendmessage = func(src, dest uint64, deployer *bind.TransactOpts) (*onramp.OnRampCCIPMessageSent, error) { | ||
return testhelpers.DoSendRequest( | ||
t, | ||
e, | ||
state, | ||
testhelpers.WithSender(deployer), | ||
testhelpers.WithSourceChain(src), | ||
testhelpers.WithDestChain(dest), | ||
testhelpers.WithTestRouter(false), | ||
testhelpers.WithEvm2AnyMessage(router.ClientEVM2AnyMessage{ | ||
Receiver: common.LeftPadBytes(state.Chains[chainB].Receiver.Address().Bytes(), 32), | ||
Data: []byte("hello"), | ||
TokenAmounts: nil, | ||
FeeToken: common.HexToAddress("0x0"), | ||
ExtraArgs: nil, | ||
})) | ||
} | ||
|
||
assertSendRequestReverted = func(src, dest uint64, deployer *bind.TransactOpts) { | ||
_, err = sendmessage(src, dest, deployer) | ||
require.Error(t, err) | ||
require.Contains(t, err.Error(), "execution reverted") | ||
} | ||
|
||
assertRequestSent = func(src, dest uint64, deployer *bind.TransactOpts) { | ||
latestHeader, err := e.Chains[dest].Client.HeaderByNumber(testcontext.Get(t), nil) | ||
require.NoError(t, err) | ||
block := latestHeader.Number.Uint64() | ||
messageSentEvent, err := sendmessage(src, dest, e.Chains[src].DeployerKey) | ||
require.NoError(t, err) | ||
expectedSeqNumExec[testhelpers.SourceDestPair{ | ||
SourceChainSelector: src, | ||
DestChainSelector: dest, | ||
}] = []uint64{messageSentEvent.SequenceNumber} | ||
startBlocks[dest] = &block | ||
} | ||
) | ||
|
||
// disable lane A -> B | ||
testhelpers.RemoveLane(t, &tenv, chainA, chainB, false) | ||
// send a message to confirm it is reverted between chainA and chainB | ||
assertSendRequestReverted(chainA, chainB, e.Chains[chainA].Users[0]) | ||
// send a message between chainB and chainA to confirm it is not reverted | ||
assertRequestSent(chainB, chainA, e.Chains[chainB].Users[0]) | ||
// disable lane B -> A | ||
testhelpers.RemoveLane(t, &tenv, chainB, chainA, false) | ||
assertSendRequestReverted(chainB, chainA, e.Chains[chainB].Users[0]) | ||
|
||
// send message in other lanes and ensure they are delivered | ||
go func() { | ||
assertRequestSent(chainA, chainC, e.Chains[chainA].Users[1]) | ||
assertRequestSent(chainC, chainA, e.Chains[chainC].Users[1]) | ||
assertRequestSent(chainB, chainC, e.Chains[chainB].Users[1]) | ||
assertRequestSent(chainC, chainB, e.Chains[chainC].Users[1]) | ||
}() | ||
// disable lanes between A & C and C & B while requests are getting sent | ||
testhelpers.RemoveLane(t, &tenv, chainA, chainC, false) | ||
b-gopalswami marked this conversation as resolved.
Show resolved
Hide resolved
|
||
testhelpers.RemoveLane(t, &tenv, chainC, chainA, false) | ||
testhelpers.RemoveLane(t, &tenv, chainB, chainC, false) | ||
testhelpers.RemoveLane(t, &tenv, chainC, chainB, false) | ||
b-gopalswami marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// check fee quoter returns error when the lane is disabled | ||
gp, err := state.Chains[chainA].FeeQuoter.GetTokenAndGasPrices(&bind.CallOpts{ | ||
Context: tests.Context(t), | ||
}, state.Chains[chainB].Weth9.Address(), chainB) | ||
require.Error(t, err) | ||
require.Contains(t, err.Error(), "execution reverted") | ||
require.Nil(t, gp.GasPriceValue) | ||
require.Nil(t, gp.TokenPrice) | ||
// confirm that message sent in all lanes are reverted after disabling the lanes | ||
for _, pair := range pairs { | ||
assertSendRequestReverted(pair.SourceChainSelector, pair.DestChainSelector, e.Chains[pair.SourceChainSelector].Users[0]) | ||
} | ||
// re-enable all the lanes | ||
var ( | ||
linkPrice = deployment.E18Mult(100) | ||
wethPrice = deployment.E18Mult(4000) | ||
) | ||
for _, pair := range pairs { | ||
testhelpers.AddLane(t, &tenv, pair.SourceChainSelector, pair.DestChainSelector, false, | ||
map[uint64]*big.Int{ | ||
pair.DestChainSelector: testhelpers.DefaultGasPrice, | ||
}, | ||
map[common.Address]*big.Int{ | ||
state.Chains[pair.SourceChainSelector].LinkToken.Address(): linkPrice, | ||
state.Chains[pair.SourceChainSelector].Weth9.Address(): wethPrice, | ||
}, | ||
changeset.DefaultFeeQuoterDestChainConfig(true)) | ||
} | ||
// send a message in all the lane including re-enabled lanes | ||
for _, pair := range pairs { | ||
assertRequestSent(pair.SourceChainSelector, pair.DestChainSelector, e.Chains[pair.SourceChainSelector].Users[0]) | ||
} | ||
// confirm all messages are delivered | ||
testhelpers.ConfirmExecWithSeqNrsForAll(t, e, state, expectedSeqNumExec, startBlocks) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
router can't be 0x0 address as Offramp expects valid router address ow it will throw ZeroAddressNotAllowed error message.
chainlink/contracts/src/v0.8/ccip/offRamp/OffRamp.sol
Line 994 in ba148ac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not possible to disable a router in offRamp, the parameter IsEnabled should be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is still needed to make here
chainlink/deployment/ccip/changeset/cs_chain_contracts.go
Line 718 in 48c7cb7