Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Stake pool DB worker doesn't save sync progress in Byron #1963

Closed
Anviking opened this issue Jul 27, 2020 · 4 comments · Fixed by #2138
Closed

Stake pool DB worker doesn't save sync progress in Byron #1963

Anviking opened this issue Jul 27, 2020 · 4 comments · Fixed by #2138
Assignees
@hasufell
Labels
Bug PRIORITY:LOW Would eventually require attention if times allow it. SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation.

Comments

@Anviking
Copy link
Member

Anviking commented Jul 27, 2020 •
edited by hasufell
Loading

Context

Information -
Version
Platform
Installation

Steps to Reproduce

  1. Start a cardano-node connected to the byron mainnet
  2. cardano-wallet-shelley serve --node-socket socket --mainnet --database wallet-db
  3. wait 20 s
  4. ctrl-c
  5. cardano-wallet-shelley serve --node-socket socket --mainnet --database wallet-db

Expected behavior

The sync progress from the first launch is saved, and it doesn't have to start from the beginning after quitting.

Actual behavior

It restarts from genesis the second time.

Syncing through the whole of byron again may take some time. I guess it takes ~10 minutes on my machine, if the node is already synced, so while not optimal, I believe it should be fine for now.

Resolution

QA

Currently there are no integration tests. All we do is test the putHeaders and listHeaders DBLayer functions: https://github.com/input-output-hk/cardano-wallet/pull/2138/files#diff-c3e090fb717915e32577d1c7311e8ab5392c830dec9a705e025f0bf84af6b099

This tests the new table block_headers.

Testing that the wallet resumes the chain at a certain point is very easy on testnet though. Starting sync process, crtl+c and start wallet again. Then check that it resumes on the previous block height.
@KtorZ KtorZ added Bug SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation. PRIORITY:LOW Would eventually require attention if times allow it. and removed BUG:MINOR labels Aug 12, 2020
@hasufell hasufell self-assigned this Sep 11, 2020
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 14, 2020
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
d78cb15
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 14, 2020
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
8c31417
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 14, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
92979ae 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 14, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
a1ddca9 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
@piotr-iohk piotr-iohk mentioned this issue Sep 14, 2020
Merged
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 15, 2020
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
daf5fb3
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 15, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
00a2823 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 15, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
57f025a 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 15, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
f017ee3 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 15, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
48376d7 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 16, 2020
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
414e2fa
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 16, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
c5bb50e 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 17, 2020
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
8e3cfd4
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 17, 2020
Record byron headers in stake pools DB and use them as sync point wrt c…
543f583 
…ardano-foundation#1963

This will allow to cancel syncing during byron era
for non-mainnet networks and pick up where we left off.

We use the same method for shelley as well instead of
using stake pool production table to unify behavior.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 17, 2020
Record headers in stake pools DB and use them as sync point wrt carda…
3368ec3 
…no-foundation#1963

This will allow to cancel syncing during byron era
for all networks and pick up where we left off.

We use the same method for shelley as well instead of
using stake pool production table to unify behavior.
hasufell pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 17, 2020
Record headers in stake pools DB and use them as sync point wrt carda…
9d1bf75 
…no-foundation#1963

This will allow to cancel syncing during byron era
for all networks and pick up where we left off.

We use the same method for shelley as well instead of
using stake pool production table to unify behavior.
KtorZ pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 22, 2020
@KtorZ
Sync from Shelley hard fork on mainnet wrt cardano-foundation#1963
005eea6
KtorZ pushed a commit to hasufell/cardano-wallet that referenced this issue Sep 22, 2020
@KtorZ
Record headers in stake pools DB and use them as sync point wrt carda…
bf6d782 
…no-foundation#1963

This will allow to cancel syncing during byron era
for all networks and pick up where we left off.

We use the same method for shelley as well instead of
using stake pool production table to unify behavior.
@hasufell
Copy link
Contributor

Why is this in progress?

@KtorZ
Copy link
Member

KtorZ commented Oct 26, 2020

@hasufell good question, why is it not in QA ?

@hasufell hasufell linked a pull request Oct 26, 2020 that will close this issue
Save intermediate state when restoring the Byron chain in the stake pools worker #2138
Merged
@hasufell
Copy link
Contributor

@piotr-iohk added QA section... I believe you already tested this, but we didn't follow the process properly I think

@piotr-iohk piotr-iohk mentioned this issue Oct 26, 2020
Merged
@piotr-iohk
Copy link
Contributor

LGTM. I have added manual test for this in #2271 / 466a43b.

iohk-bors bot added a commit that referenced this issue Oct 27, 2020
@iohk-bors
Merge #2271 #2274
5573ad8 
2271: Revise manual tests r=piotr-iohk a=piotr-iohk

# Issue Number
Somewhat related to #1963.

# Overview

- dac491d
  jormungandr specific tests
  
- 9b0c551
  Rewrite DBMigration and SyncTolerance tests to be run on cardano-node
  
- 2db0905
  Drop redundant SyncProgress and TrackBlockHeight tests (sanity tested during DBMigration)
  
- 466a43b
  Test for Syncing in the Byron era
  
- 143f2cb
  Add $ in from of cmds and fix indentation
  
- 2c40953
  Add exemplary curl commands for smoke checking wallet functionality in DBMigration test




# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


2274: More details on index param for `Sign metadata` and `Get public key` endpoints in swagger. r=piotr-iohk a=piotr-iohk

# Issue Number

ADP-509 / ADP-474

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- 9c6dd06
  More details on index param for `Sign metadata` and `Get public key` endpoints in swagger.

# Comments

![Screenshot from 2020-10-27 10-36-55](https://user-images.githubusercontent.com/42900201/97283754-88fa2300-1840-11eb-9586-8be33267dd69.png)



Co-authored-by: Piotr Stachyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hasufell hasufell

Labels
Bug PRIORITY:LOW Would eventually require attention if times allow it. SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation.
Projects
None yet
Milestone
No milestone
4 participants
@Anviking @hasufell @KtorZ @piotr-iohk