move dcr related functionality to it's own package #257
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package dcrlibwallet | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
|
|
||
| "decred.org/dcrwallet/v2/errors" | ||
|
|
||
| "github.com/asdine/storm" | ||
|
|
||
| bolt "go.etcd.io/bbolt" | ||
|
|
||
| "github.com/planetdecred/dcrlibwallet/wallets/dcr" | ||
| ) | ||
|
|
||
| func initializeDCRWallet(rootDir, dbDriver, netType string) (*storm.DB, string, error) { | ||
| var mwDB *storm.DB | ||
|
There was a problem hiding this comment. you could drop this and instead make the following change at line 30. |
||
|
|
||
| rootDir = filepath.Join(rootDir, netType, "dcr") | ||
| err := os.MkdirAll(rootDir, os.ModePerm) | ||
| if err != nil { | ||
| return mwDB, "", errors.Errorf("failed to create dcr rootDir: %v", err) | ||
|
There was a problem hiding this comment. Since There was a problem hiding this comment. returning nil would give you an error in that instance which is why I initialized an empty instance at the top |
||
| } | ||
|
|
||
| err = initLogRotator(filepath.Join(rootDir, logFileName)) | ||
| if err != nil { | ||
| return mwDB, "", errors.Errorf("failed to init dcr logRotator: %v", err.Error()) | ||
| } | ||
|
|
||
| mwDB, err = storm.Open(filepath.Join(rootDir, walletsDbName)) | ||
|
There was a problem hiding this comment.
|
||
| if err != nil { | ||
| log.Errorf("Error opening dcr wallets database: %s", err.Error()) | ||
| if err == bolt.ErrTimeout { | ||
| // timeout error occurs if storm fails to acquire a lock on the database file | ||
| return mwDB, "", errors.E(ErrWalletDatabaseInUse) | ||
| } | ||
| return mwDB, "", errors.Errorf("error opening dcr wallets database: %s", err.Error()) | ||
| } | ||
|
|
||
| // init database for saving/reading wallet objects | ||
| err = mwDB.Init(&dcr.Wallet{}) | ||
| if err != nil { | ||
| log.Errorf("Error initializing wallets database: %s", err.Error()) | ||
| return mwDB, "", err | ||
| } | ||
|
|
||
| // init database for saving/reading proposal objects | ||
| err = mwDB.Init(&dcr.Proposal{}) | ||
| if err != nil { | ||
| log.Errorf("Error initializing wallets database: %s", err.Error()) | ||
| return mwDB, "", err | ||
| } | ||
|
|
||
| return mwDB, rootDir, nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really initialize a dcr wallet; the databases initialized here aren't specifically used for a single dcr wallet, one is used to store multiwallet data including ALL (dcr) wallets info and also general, non-wallet specific config data. This is really actually initializing the multiwallet. It also even initializes the log rotator which is not tied to any wallet in particular but is meant for all logging done by this library. I'd say let's move the body of this function back to the
NewMultiwalletfunction, and also retain the rootDir without appending "dcr".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we'd need a function like this in the dcr package, perhaps a
InitializeWalletfunction so it reads asdcr.InitializeWallet. I'm not yet completely sure what thedcr.InitializeWalletfunction would do but it would be quite similar to what we have here. It wouldn't/shouldn't initialize a new log rotator though, the caller of the function should provide a logger to use. It also likely won't need to init a politeia database since politeia data won't be stored per wallet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appending "dcr" enables us keep all dcr database files under a single folder and seems easier to manage/track. I agree the method name is icky, and doesn't properly describe what's going on.
having separate rootdir for different assets looks advantageous in terms of folder structure
also do you think it's remotely possible we might want to be using different database drivers for different coins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. What I was getting at is, the databases created here aren't even for all dcr wallets alone but for all wallets. It's the multiwallet parent database that tracks ALL wallets for all assets, which is used to populate/restore all the asset wallets when multiwallet is initialized. It also stores general config values that aren't related to any wallet.
The "dcr" subfolder should be reserved for dcr wallet databases, as in your screenshot. But there should also exist in the rootDir, a multiwallet.db file (currently called wallets.db, as in storing info for all wallets, which makes sense too).
Yes. Very possible.