Use observed home chain reader in CCIP #15628
Conversation
mateusz-sekara
requested review from
winder,
rstout,
asoliman92,
makramkd,
dimkouv,
0xAustinWang,
b-gopalswami and
vyzaldysanchez
| t *testing.T, | ||
| cr types.ContractReader, | ||
| ccAddress common.Address, | ||
| chainID string, |
There was a problem hiding this comment.
what is chainID? (evm) Maybe we want to (also) use chainSelector?
There was a problem hiding this comment.
It could be anything that is an identifier (for evm it's going to be integer). ChainSelectors doesn't make sense for monitoring, because they are randomized and very hard to analyze dashboards with chainSelectors instead of chainID
There was a problem hiding this comment.
We already have common.Address here, so no need to make it agnostic at this stage. This is also used only in test code
| hcr := ccipreader.NewObservedHomeChainReader( | ||
| cr, | ||
| logger.TestLogger(t), | ||
| 50*time.Millisecond, |
There was a problem hiding this comment.
NIT: You can define 50ms in a const. While going through the PR I have to go the actual method to understand what this is.
There was a problem hiding this comment.
I've only fixed formatting here :P
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* Switching to the observed version * Switching to the observed version
Requires
Supports