Skip to content

Make SC wallet feature derive from any base wallet#471

Merged
quantumagi merged 3 commits intostratisproject:masterfrom
quantumagi:scoverridesanywallet
Mar 18, 2021
Merged

Make SC wallet feature derive from any base wallet#471
quantumagi merged 3 commits intostratisproject:masterfrom
quantumagi:scoverridesanywallet

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Mar 17, 2021

We must be able to use the SC wallet feature with the Strax daemon which currently implements the cold staking wallet.

This is a backwards compatible change required by system contracts.

@quantumagi quantumagi requested review from fassadlr and rowandh March 17, 2021 08:49
IWalletManager walletManager,
IWalletSyncManager walletSyncManager,
INodeStats nodeStats)
public SmartContractWalletFeature(ILoggerFactory loggerFactory)
Copy link
Contributor

Choose a reason for hiding this comment

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

You might as well remove the loggerfactory here as well and use NLog's LogManager.GetCurrentClassLogger below ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

{
LoggingConfiguration.RegisterFeatureNamespace<WalletFeature>("smart contract wallet");

if (addVanillaWallet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wont this then always be true? Why would we call this with false?

Copy link
Contributor Author

@quantumagi quantumagi Mar 18, 2021

Choose a reason for hiding this comment

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

In StraxD we would have:

.UseColdStakingWallet()
.UseSmartContractWallet(false)

Copy link
Contributor

@fassadlr fassadlr left a comment

Choose a reason for hiding this comment

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

Just remove that loggerfactory and we can approve and merge.

@quantumagi quantumagi merged commit 2f34689 into stratisproject:master Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants