Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Add a RPC syncing example. #79

Closed
wants to merge 2 commits into from

Conversation

rajarshimaitra
Copy link
Collaborator

This PR adds a RPC syncing example with "create a core wallet, and call listtransaction" approach.

Similar to electrum, RPC sync is done in two steps.

  • get all the newly added txid from and initial wallet sync. This stage also includes the reorg detection.

  • Fetch the txs not already stpred in the TxGraph and complete the final update structure.

  • Apply the update to both tracker and database.

Note: The current approach of RPC syncing with a corresponding internal
core wallet, cannot do random script pubkey scanning.

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2022

Codecov Report

Merging #79 (ff0145e) into master (cef4985) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   56.74%   56.74%           
=======================================
  Files          10       10           
  Lines         252      252           
=======================================
  Hits          143      143           
  Misses        109      109           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rajarshimaitra rajarshimaitra force-pushed the rpc-example branch 2 times, most recently from 8a79189 to abd7cde Compare December 11, 2022 14:02
@rajarshimaitra
Copy link
Collaborator Author

@evanlinjin, updated the example to work with the latest invariant around ChainGraph. The wallet_scan returns SparseChain which is then used to determine the newly found txids.. This allows us to keep the two stage syncing process working, without violating the ChainGraph restrictions..

@rajarshimaitra
Copy link
Collaborator Author

Latest push

  • Rebased on master. Fixed conflicts.
  • Modified the RPC syncing logic with the similar internal APIs used for electrum and esplora.

@evanlinjin @LLFourn This is now ready for a look..

unreachable!("We should not encounter this error as we ensured tx_height <= tip.height");
}
sparse_chain::InsertTxError::TxMovedUnexpectedly { .. } => {
/* This means there is a reorg, we will handle this situation below */
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems the situation is not handled. Maybe it is better to break here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the break happens in the next segment // Check for Reorg during the above sync process. I do think we should do better than just error in such cases. But this is how it's done for other examples, so I followed the same here too.

A few lines above, when we first check for Reorg during sync, we call the scan function recursively. Maybe we can do the same for the case after extracting the list of transactions? i.e, if there's again a reorg during sync, call scan again?

Open to suggestions here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reorg during sync is a very low probability situation, and when the user gets this error, they should just run the scan again. In that perspective, it makes sense to call the scan again internally rather than throwing the error?

Similar to electrum, RPC sync is done in two steps.

 - get all the newly added txid from and initial wallet sync.
 This stage also includes the reorg detection.

 - Fetch the txs not already stpred in the TxGraph and complete the final
 update structure.

 - Apply the update to both tracker and database.

 Note: The current approach of RPC syncing with a corresponding internal
 core wallet, cannot do random script pubkey scanning.
@rajarshimaitra
Copy link
Collaborator Author

closing in favour of #170

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants