Add DEX support for Cryptopower btc wallet #349
Conversation
ukane-philemon
force-pushed
the
add-dex-btc-support
branch
from
4319c0f to
b907c46
Compare
ukane-philemon
force-pushed
the
add-dex-btc-support
branch
from
5d13392 to
e0da68c
Compare
libwallet/assets/btc/dex-wallet.go
Outdated
| func (dw *DEXWallet) WaitForShutdown() { | ||
| dw.w.WaitForShutdown() | ||
| } |
There was a problem hiding this comment.
This method isn't needed upstream, please remove upstream and remove here as well.
If it is actually needed, we might want to do nothing here, because this call to dw.w.WaitForShutdown() will block until the wallet is stopped and there's no telling when that will happen, since the DEX client isn't allowed to stop the wallet.
There was a problem hiding this comment.
please remove upstream and remove here as well
This upstream removal will have to be a separate PR as it does not relate to customer wallet registration. We can just do nothing here, okay?
EDIT: The method is used in dex during wallet rescan, best we can do is leave empty.
| // currently unused | ||
| func (dw *DEXWallet) ChainSynced() bool { | ||
| return dw.w.ChainSynced() | ||
| } |
There was a problem hiding this comment.
Remove upstream? It can always be re-added when needed.
There was a problem hiding this comment.
Alright, It'll have to be a separate upstream PR.
libwallet/assets/btc/dex-wallet.go
Outdated
| // The below methods are not implemented by *wallet.Wallet, so must be | ||
| // implemented by the dexbtc.BTCWallet implementation. |
There was a problem hiding this comment.
This comment is no longer relevant. *DEXWallet implements all methods, even if it simply calls the same method on the underlying *wallet.Wallet.
|
|
||
| // AccountInfo returns the account information of the wallet for use by the | ||
| // exchange wallet. | ||
| func (dw *DEXWallet) AccountInfo() dexbtc.XCWalletAccount { |
There was a problem hiding this comment.
Should we perhaps split this upstream to different methods? AccountNumber() and AccountName()? How often is the account name really needed?
There was a problem hiding this comment.
3:1
Yh, we should make these separate methods.
On second thought, buck suggested calling this method once and saving the values on the upstream spv struct. Maybe we should do that instead but only for the account number, that way, we only need to call this method when we need the account name.
libwallet/assets/dcr/dex_wallet.go
Outdated
|
|
||
| unMixedAcctNum := dw.UnmixedAccountNumber() | ||
| mixedAcctNum := dw.MixedAccountNumber() | ||
| accounts, err := dw.GetAccountsRaw() |
There was a problem hiding this comment.
You shouldn't need to get all accounts information; just dw.AccountName(...)
libwallet/assets/dcr/dex_wallet.go
Outdated
| // We only care about the default account. | ||
| if mixedAccName == "" { | ||
| log.Errorf("Account name not found for mixed account number %d", mixedAcctNum) | ||
| return accts | ||
| } |
There was a problem hiding this comment.
The comment isn't clear. We only care about the default account?
There was a problem hiding this comment.
We only care about the default account?
Oh, this should say primary account.
libwallet/assets_manager.go
Outdated
| mgr.dexcMtx.Unlock() | ||
|
|
||
| err = mgr.PrepareDexSupportForDcrWallet() | ||
| // Prepare support for DCR wallet. | ||
| err = mgr.PrepareDexSupportForDCRWallet() |
There was a problem hiding this comment.
Can this mgr.InitializeDEX method be called multiple times while the app is open? If not, then mgr.PrepareDexSupportForDCRWallet() should not need to worry about duplicate registrations. Also, I don't see why we would want to initialize dexc multiple times.
There was a problem hiding this comment.
Can this mgr.InitializeDEX method be called multiple times while the app is open?
Yes. We can't control what happens in dex core so we try to restart the dex client if it's not yet started(or it started but then exited).
Testing on #353? Most fixes on the dex bonding page have been handled there. This PR only focuses on btc. |
No here. I'll try again there. Feel free to ignore here. |
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
…tor in go mod Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
ukane-philemon
force-pushed
the
add-dex-btc-support
branch
from
9eaeceb to
fd6fc43
Compare
Closes #337
EDIT: This PR uses
github.com/ukane-philemon/dcrdex/add-custom-wallet-constructoras a temp replacement ofdecred/dcrdex