-
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
add vrf replay logs #12887
add vrf replay logs #12887
Conversation
I see you updated files related to
|
"numBlocksToReplay", numBlocksToReplay, | ||
"replayStartBlock", replayStartBlock, | ||
"requestTimeout", lsn.job.VRFSpec.RequestTimeout, | ||
"finalizedLatestBlock", latestBlock.FinalizedBlockNumber, |
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.
Would this already be added by line 123?
ll := lsn.l.With(
"latestFinalizedBlock", latestBlock.FinalizedBlockNumber,
"latestBlock", latestBlock.BlockNumber,
"fromTimestamp", fromTimestamp)
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.
right. let me remove the overlapping ones.
I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:
|
case "1": // eth mainnet | ||
case "3": // eth ropsten | ||
case "4": // eth rinkeby | ||
case "5": // eth goerli | ||
case "11155111": // eth sepolia | ||
case "1", "3", "4", "5", "11155111": // eth mainnet, robsten, rinkeby, goerli, sepolia |
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 kind of like the previous style of 1 chainId per line. It makes things much easier to read.
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 learned previous style is wrong. all of these went to default:
case "1": // eth mainnet
case "3": // eth ropsten
case "4": // eth rinkeby
case "5": // eth goerli
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.
Ah. This style only works for languages with explicit break
. Is it possible to something like below?
case
"1", // eth mainnet
"3", // eth ropsten
"4", // eth rinkeby
"5": // eth goerli
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.
sure can change it
core/services/vrf/delegate.go
Outdated
if err != nil { | ||
return nil, err | ||
} | ||
d.lggr.Debugw("Creating services for job spec", "job", string(marshalledJob)) |
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.
Can consider adding more tags like key hashes, max gas price, etc to make it easier to find within the logs.
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.
makes sense. so key hash can be derived from "uncompressedKey" in the job spec. max gas price is already being printed elsewhere: Key = '0xd13b46290f61dB3E6638bE2746e88F0c04AB883F'\n\n[EVM.KeySpecific.GasEstimator]\nPriceMax = '200 gwei'
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.
Right. Anything printed can be found as a regex matching the log since tags are not indexed anyway.
Quality Gate passedIssues Measures |
No description provided.