CCIP-4948: Adding new test for disable and enable lane #16038
Conversation
| @@ -705,12 +705,10 @@ func UpdateOffRampSourcesChangeset(e deployment.Environment, cfg UpdateOffRampSo | |||
| var args []offramp.OffRampSourceChainConfigArgs | |||
| for source, update := range updates { | |||
| router := common.HexToAddress("0x0") | |||
| if update.IsEnabled { | |||
There was a problem hiding this comment.
router can't be 0x0 address as Offramp expects valid router address ow it will throw ZeroAddressNotAllowed error message.
There was a problem hiding this comment.
If it's not possible to disable a router in offRamp, the parameter IsEnabled should be removed
There was a problem hiding this comment.
I think it is still needed to make here
b-gopalswami
requested review from
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang and
0xnogo
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
b-gopalswami
force-pushed
the
ccip-4948
branch
from
457f900 to
093222d
Compare
| @@ -705,12 +705,10 @@ func UpdateOffRampSourcesChangeset(e deployment.Environment, cfg UpdateOffRampSo | |||
| var args []offramp.OffRampSourceChainConfigArgs | |||
| for source, update := range updates { | |||
| router := common.HexToAddress("0x0") | |||
| if update.IsEnabled { | |||
There was a problem hiding this comment.
If it's not possible to disable a router in offRamp, the parameter IsEnabled should be removed
review comments rename Add test to CI and add enabledConfig flag remove unwanted sql add variables locally
b-gopalswami
force-pushed
the
ccip-4948
branch
from
8ad49b2 to
2dbae80
Compare
| 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.
@makramkd would it affect the inflight requests?
There was a problem hiding this comment.
Yeah, no offramp changes should be made
There was a problem hiding this comment.
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.
if no offramp changes are made, then in-flight requests are getting delivered.
Yeah, just to be clear this is the desired behavior.
remove if
b-gopalswami
force-pushed
the
ccip-4948
branch
from
9b9b48a to
193de02
Compare
|
Adding test to verify the op changeset workflow to disable and re-enable the lane.
Tests it will cover: TC001 & TC002
https://smartcontract-it.atlassian.net/wiki/spaces/QA/pages/1262846210/CCIP+V1.6+Changeset+Test+Plan