LogPoller CLI command to resolve reorg greater than finality depth #12867
Conversation
|
I see you updated files related to
|
|
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:
|
core/cmd/blocks_commands.go
Outdated
| @@ -34,6 +36,18 @@ func initBlocksSubCmds(s *Shell) []cli.Command { | |||
| }, | |||
| }, | |||
| }, | |||
| { | |||
| Name: "find_lca", | |||
There was a problem hiding this comment.
is there a convention for when to use _ vs - for clis? Not sure if there is but was curious for the reason for the difference in naming between find_lca and remove-blocks
core/web/lca_controller.go
Outdated
| "net/http" | ||
|
|
||
| "github.com/gin-gonic/gin" | ||
| "github.com/pkg/errors" |
There was a problem hiding this comment.
if possible can we use the standard errors package instead of pkg/errors. I think we can here. We are trying to reduce the use of this dependency since it is no longer maintained
| --- | ||
|
|
||
| Added a new CLI command, `blocks find_lca,` which finds the latest block that is available in both the database and on the chain for the specified chain. | ||
| Added a new CLI command, `node remove-blocks,` which removes all blocks and logs greater than or equal to the specified block number. |
There was a problem hiding this comment.
What was the rationale behind placing one of these commands under blocks and the other one under node. Seems like it might be more difficult to find them both if they're not in the same place?
Because these are both evm-specific commands, I think they should probably both go under chainlink chains evm... although I do notice there are some commands, such as chainlink blocks replay which are also evm-specific.
(We should probably move that one under chainlink chains evm soon also. I'll mention that to those more involved with LOOP arch planning.)
There was a problem hiding this comment.
I agree that it's confusing. I've just tried to follow existing structure of the commands.
Existing blocks replay interacts with the LogPoller and triggers it to replay blocks. find-lca - also interacts with the LogPoller and requests latest good block. Thus it made sense to me to group them together.
At the same time chains evm simply lists available evm chains.
Initially I've planed to have remove-blocks in the blocks section, but realised that it needs unique lock on the database. node/local sections seems to contain all commands that must be executed on local node only. It also contains rebroadcast-transactions, which is EVM/TXM related but still resides here. I figured that all local commands must be grouped together to highlight that they are executed on the local node.
Foundations are the owners of this package, so It's up to you to decide where to put the commands.
Should I move them?
There was a problem hiding this comment.
Yeah, given that one of them is local and the other isn't I agree that it makes sense to have remove-blocks with rebroadcast-transactions and find-lca with replay. Unfortunately, I think we're going to want to move remove-blocks and rebroadcast-transactions soon to be under chainlink blocks evm and chainlink node evm. So I'm not sure whether it's better to add them now in the same place and move them later, or just add them in the right place now and move the other two later. Asking rest of my team for some advice on that.
There was a problem hiding this comment.
@dhaidashenko Alright, yeah let's just add them here now and we can move them all together later.
|
I tried running this, and noticed that I wonder if the second one is too limiting though. I was thinking deleting recent blocks from (Referring to db lease lock error message, when I try to run it while the node is running.) |
dhaidashenko
dismissed stale reviews from prashantkumar1982, reductionista, and poopoothegorilla
via
72dc7f5
|
…12867) * find lca and remove block after CLI * fix sort.Find typo * make RemoveBlocks local cmd * tests * added changeset * added tags to the changeset * fixed tests * make cmds, vars cases consistent
* implement update node operators in capability registry * address PR feedback * cache owner call * Validate config on NewApplication (#12997) * validate config on new application spawn * Add changeset * Check config only in the rebroadcast-transactions command * Configurable Mercury transmitter parameters (#12680) * Configurable Mercury transmitter parameters * Changeset * Remove commented code * add tag * Rename * LogPoller CLI command to resolve reorg greater than finality depth (#12867) * find lca and remove block after CLI * fix sort.Find typo * make RemoveBlocks local cmd * tests * added changeset * added tags to the changeset * fixed tests * make cmds, vars cases consistent * Fix Node Migration Test Check For Versions (#12982) * fix: prevent query syntax error if allowlist is empty (#12912) Co-authored-by: Morgan Kuphal <[email protected]> * Update wrappers * Formatting --------- Co-authored-by: george-dorin <[email protected]> Co-authored-by: Sam <[email protected]> Co-authored-by: Dmytro Haidashenko <[email protected]> Co-authored-by: Tate <[email protected]> Co-authored-by: Gabriel Paradiso <[email protected]> Co-authored-by: Morgan Kuphal <[email protected]> Co-authored-by: DeividasK <[email protected]>
Added new commands:
blocks find_lca- finds the latest block that is available in both the database and on the chain for the specified chain.node remove-blocksremoves all blocks and logs greater than or equal to the specified block number.node remove-blocksis a local command and locks the database to ensure there is no data races within LogPoller or it's clients.