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.

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

move dcr related functionality to it's own package #257

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Next Next commit
move dcr related capability to the dcr package
  • Loading branch information
dreacot committed Jul 21, 2022
commit 2b0f8106e8edb9f4bcf09feb5c7228b5a4b6d4d5
58 changes: 58 additions & 0 deletions dcr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package dcrlibwallet

import (
// "context"
// "fmt"
"os"
"path/filepath"

"decred.org/dcrwallet/v2/errors"

"github.com/asdine/storm"
// "github.com/asdine/storm/q"

bolt "go.etcd.io/bbolt"

"github.com/planetdecred/dcrlibwallet/wallets/dcr"
)

func initializeDCRWallet(rootDir, dbDriver, netType string) (*storm.DB, string, error) {
Copy link
Contributor

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 NewMultiwallet function, and also retain the rootDir without appending "dcr".

Copy link
Contributor

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 InitializeWallet function so it reads as dcr.InitializeWallet. I'm not yet completely sure what the dcr.InitializeWallet function 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.

Copy link
Contributor Author

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
Screenshot from 2022-08-23 19-47-46

also do you think it's remotely possible we might want to be using different database drivers for different coins?

Copy link
Contributor

Choose a reason for hiding this comment

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

having separate rootdir for different assets looks advantageous in terms of folder structure

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).

do you think it's remotely possible we might want to be using different database drivers for different coins?

Yes. Very possible.

var mwDB *storm.DB
Copy link

Choose a reason for hiding this comment

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

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)
Copy link

Choose a reason for hiding this comment

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

Since mwDB is not assigned, wouldn't it be easier to return nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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))
Copy link

Choose a reason for hiding this comment

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

mwDB, err := storm.Open(filepath.Join(rootDir, walletsDbName))

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
}
2 changes: 1 addition & 1 deletion dexclient.go
Original file line number Diff line number Diff line change
@@ -105,7 +105,7 @@ func (mw *MultiWallet) prepareDexSupportForDcrWalletLibrary() error {
return nil, fmt.Errorf("account error: %v", err)
}

walletDesc := fmt.Sprintf("%q in %s", wallet.Name, wallet.dataDir)
walletDesc := fmt.Sprintf("%q in %s", wallet.Name, wallet.DataDir)
return dexdcr.NewSpvWallet(wallet.Internal(), walletDesc, chainParams, logger.SubLogger("DLWL")), nil
}

8 changes: 8 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -5,6 +5,12 @@ require (
decred.org/dcrwallet/v2 v2.0.2-0.20220505152146-ece5da349895
github.com/DataDog/zstd v1.4.8 // indirect
github.com/asdine/storm v0.0.0-20190216191021-fe89819f6282
github.com/btcsuite/btcd v0.22.0-beta.0.20211026140004-31791ba4dc6e // indirect
github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f // indirect
github.com/btcsuite/btcutil v1.0.3-0.20210527170813-e2ba6805a890 // indirect
github.com/btcsuite/btcwallet v0.12.0 // indirect
github.com/btcsuite/btcwallet/walletdb v1.4.0 // indirect
github.com/btcsuite/btcwallet/wtxmgr v1.3.0 // indirect
github.com/companyzero/sntrup4591761 v0.0.0-20220309191932-9e0f3af2f07a // indirect
github.com/dchest/siphash v1.2.3 // indirect
github.com/decred/base58 v1.0.4 // indirect
@@ -16,6 +22,7 @@ require (
github.com/decred/dcrd/dcrutil/v4 v4.0.0
github.com/decred/dcrd/gcs/v3 v3.0.0
github.com/decred/dcrd/hdkeychain/v3 v3.1.0
github.com/decred/dcrd/rpc/jsonrpc/types/v3 v3.0.0
github.com/decred/dcrd/txscript/v4 v4.0.0
github.com/decred/dcrd/wire v1.5.0
github.com/decred/dcrdata/v7 v7.0.0-20211216152310-365c9dc820eb
@@ -26,6 +33,7 @@ require (
github.com/jessevdk/go-flags v1.5.0 // indirect
github.com/jrick/logrotate v1.0.0
github.com/kevinburke/nacl v0.0.0-20190829012316-f3ed23dbd7f8
github.com/lightninglabs/neutrino v0.13.1-0.20211214231330-53b628ce1756 // indirect
github.com/onsi/ginkgo v1.14.0
github.com/onsi/gomega v1.10.1
github.com/planetdecred/dcrlibwallet/dexdcr v0.0.0-20220223161805-c736f970653d
Loading