dcrwallet icon indicating copy to clipboard operation
dcrwallet copied to clipboard

udb: add upgradeToVersion12

Open kLkA opened this issue 7 years ago • 11 comments

#1036

In this upgrade function we do check all MSOs for related tx existence. If TX doesn't exist then we drop MSO

kLkA avatar May 03 '18 19:05 kLkA

Few issues:

A long time ago we had several different components (address manager, tx store, stake store) all being saved in the database, but they were versioned separately and one component could not use data from another because it could not assume the data format. This was fixed by moving everything under the udb package and merging all of the components under a single database version. The old upgrade code for each individual component was still necessary to bring it up to the state where the merge could be performed, but following that, no more upgrades are made to the components individually. See upgrades.go, upgrades_test.go, and the testdata directory for the new upgrade mechanism and testing infrastructure.

Second, you can not delete or put values when using a cursor or within a ForEach function (which also wraps a cursor). This invalidates the cursor and further cursor usage can silently cause database corruption. Read all values which need changes first and then modify them at the end.

jrick avatar May 04 '18 13:05 jrick

Also, be aware of golang/go#18007 when generating the testdata files. I have needed to rename testdata to something else just to go run the generators.

jrick avatar May 04 '18 13:05 jrick

I didn't get why to generate new testdata since it is only data inconsistency fix. It should be fully compatible with v7 version, isn't it?

kLkA avatar May 04 '18 19:05 kLkA

After upgrading, the database will be compatible with the older db test artifacts, however those won't expose the problem we are trying to fix here.

jrick avatar May 07 '18 14:05 jrick

Hi @jrick . I was exploring methods to create mock objects in multisigOut bucket and didn't find any public functions for it. Could you clarify that thing and maybe suggest some approach you would like to see?

kLkA avatar May 07 '18 21:05 kLkA

@jrick Travis CI error log doesn't seem like related to new code

kLkA avatar May 08 '18 21:05 kLkA

The error is fixed on master now.

As for creating the test db, you can just write keys/values directly to the DB. This is fine for this case since we are intentionally trying to create an invalid DB that will be fixed during the upgrade.

jrick avatar May 09 '18 14:05 jrick

Is it ok If I will leave more complex solution for invalid DB creation? I didn't manage to create valid value of multisig record without using multisig methods

kLkA avatar May 09 '18 16:05 kLkA

The invalid DB for the test can be created in a more simpler way by using keys and values created from the keyMultisigOut and valueMultisigOut functions. Since these are unexported, you can copy them into v9.go. The best test would contain multisig outputs with and without the associated transaction, so we can test that only those outputs without the transaction are removed.

jrick avatar May 11 '18 18:05 jrick

Changed approach to use keyMultisigOut and valueMultisigOut functions. Added multisig outputs with and without associated transaction

kLkA avatar May 11 '18 21:05 kLkA

Done

kLkA avatar Oct 11 '18 19:10 kLkA