Skip to content

Comments

pc/wallet rescan#486

Closed
paolocappelletti wants to merge 18 commits intodevfrom
pc/wallet-rescan
Closed

pc/wallet rescan#486
paolocappelletti wants to merge 18 commits intodevfrom
pc/wallet-rescan

Conversation

@paolocappelletti
Copy link
Collaborator

@paolocappelletti paolocappelletti commented Aug 3, 2022

This PR adds reindex feature to a sidechain node.
The reindex resets state and wallet (except the secret storage) to the initial state , then all the blocks from the history are applied again one by one to them.
Reindexing is performed in an asynch way, so that the node is not blocked during the process.
All "write" operations (send of transactions, forging, tx or blocks received by other nodes) are not executed during the reindex.
All "read" operations, including sending local blocks to other peers, are allowed.

Two new endpoints has been added to the wallet/ , to launch the reindex and get info about its progress.
Furthermore, a new debug endpoint is added only in regtest, to allow to perform step by step the reindex, for debug pourposes

If the node is stopped during reindex, it will restart from the reached height at the next restart.

Mode details here: https://www.notion.so/horizen-labs/Wallet-rescan-d2ffe9298bfd468681088977cb4095d4

related PR: HorizenOfficial/developer-horizen-global#69

@paolocappelletti paolocappelletti changed the title [work in progress] pc/wallet rescan pc/wallet rescan Aug 5, 2022
@i-Alex i-Alex requested a review from sgonta August 18, 2022 07:03
val sidechainTransactionApiRoute: Route = SidechainTransactionApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, mockedSidechainTransactionActorRef,
sidechainTransactionsCompanion, params).route
val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, sidechainSecretsCompanion).route
val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, null, sidechainSecretsCompanion).route
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please mark variable, what is null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the null variable is the new SidechainNodeViewReindexer, since it is not used in this test I did not mocked it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess Sergiy meant something like val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, sidechainNodeViewReindexer=null, sidechainSecretsCompanion).route
to clearly understand what was nullified.

Copy link
Contributor

@MarcoOl94 MarcoOl94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. Just few comments:

  1. Maybe it can be good to check if the reindex is in progress also in the endpoints that use the StateStorage (e.g. hashCeased, getSidechainBlockIdForBackup).
  2. There can be issues in case of having a reindex in progress at the same time of a Certificate generation?
  3. Do we want to add also an optional parameter to the endpoints importSecret and importSecrets that can launch automatically a reindex (defualt=False)?

Copy link
Contributor

@i-Alex i-Alex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tricky feature, a lot of curvy points there.
The overall design is very good!

val sidechainTransactionApiRoute: Route = SidechainTransactionApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, mockedSidechainTransactionActorRef,
sidechainTransactionsCompanion, params).route
val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, sidechainSecretsCompanion).route
val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, null, sidechainSecretsCompanion).route
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess Sergiy meant something like val sidechainWalletApiRoute: Route = SidechainWalletApiRoute(mockedRESTSettings, mockedSidechainNodeViewHolderRef, sidechainNodeViewReindexer=null, sidechainSecretsCompanion).route
to clearly understand what was nullified.

@paolocappelletti
Copy link
Collaborator Author

hope to have collected all the cases.. see also here @i-Alex :
https://docs.google.com/document/d/1Ipn4umbwVqJep-UfS5gdM7HSKnbCcK6JVGiJRLBIy8c/edit#

@paolocappelletti paolocappelletti added WONT DO Old PRs decommissioned and not merged and removed DO NOT MERGE labels Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WONT DO Old PRs decommissioned and not merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants