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

[MultiSig] Dust inputs should not be indexed/added in the MS wallet #529

Merged
merged 4 commits into from
Apr 23, 2021

Conversation

fassadlr
Copy link
Contributor

Please review carefully guys 👍

@quantumagi
Copy link
Contributor

quantumagi commented Apr 22, 2021

Copy link
Contributor Author

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

Is this affected by this change? It appears to be unnecessary now:
https://github.com/stratisproject/StratisFullNode/pull/520/files#diff-b695747a4e647b99d1cf6096d408b5135e319eef484ed4c1935f9b58c3c39364R305-R306

@quantumagi that code is still required as the MS wallet still contains those dust like utxos. So we still have to filter them out.

Copy link
Contributor

@noescape00 noescape00 left a comment

Choose a reason for hiding this comment

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

LGTM

@quantumagi
Copy link
Contributor

quantumagi commented Apr 22, 2021

@quantumagi that code is still required as the MS wallet still contains those dust like utxos. So we still have to filter them out.

@fassadlr , @noescape00, doesn't the index you're reading the utxo's from already filter by IsSpendable as in:

                    if (transactionData.IsSpendable())
                        this.spendableTransactionList.Add(transactionData, transactionData);

so when you do this, its's really not necessary:

                IList<TransactionData> result = this.spendableTransactionList.Keys;

                if (filterDustTransactions)
                    result = result.Where(x => x.Amount > Money.Coins(FederatedPegSettings.UtxoAmountThreshold)).ToArray();

considering:

      public bool IsSpendable()
        {
            if (this.Amount < Money.Coins(FederatedPegSettings.UtxoAmountThreshold))
                return false;

            // TODO: Coinbase maturity check?
            return this.SpendingDetails == null;
        }

Copy link
Contributor

@quantumagi quantumagi left a comment

Choose a reason for hiding this comment

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

LGTM

@fassadlr fassadlr merged commit 103df9e into stratisproject:master Apr 23, 2021
@fassadlr fassadlr deleted the dustaddfix branch April 23, 2021 08:49
fassadlr added a commit to zeptin/StratisFullNode that referenced this pull request May 27, 2021
…tratisproject#529)

* Dust inputs are not added to the multisig wallet

* Review changes

* Remove extra filter
zeptin pushed a commit to zeptin/StratisFullNode that referenced this pull request Jun 15, 2021
…tratisproject#529)

* Dust inputs are not added to the multisig wallet

* Review changes

* Remove extra filter
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.

3 participants