-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Streamline, refactor and unify PS checks for mixing entries and final txes #3246
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
Conversation
Also streamline logic a bit and drop unused/excessive parts.
A tx with duplicate inputs would be considered invalid anyway and we also know there are no duplicates because we just verified the final tx above. Also drop an unused variable.
Co-Authored-By: PastaPastaPasta <[email protected]>
codablock
left a comment
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.
See inline comments
…ssion::SignFinalTransaction() and CPrivateSendServer::AddEntry() into CPrivateSendBaseSession::IsValidInOuts()
codablock
left a comment
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.
see inline comments
|
👍 Applied proposed fixes |
codablock
left a comment
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.
utACK
|
POST-ACK tested on testnet and everything seemed normal. |
… txes (dashpay#3246) * Move PS mixing entry verification on masternodes into `AddEntry()` Also streamline logic a bit and drop unused/excessive parts. * Unify PS checks among masternodes and clients * No need to re-check outputs over and over again * No need to count, fail early if any output is missing * No need to look any further once we found the input we expected A tx with duplicate inputs would be considered invalid anyway and we also know there are no duplicates because we just verified the final tx above. Also drop an unused variable. * Unify LogPrint-s * Drop human-readable strings for unused PoolMessage-s * Apply suggestions from code review Co-Authored-By: PastaPastaPasta <[email protected]> * Re-introduce zero-fee checks * fix log * Move all txin/txout verification logic shared by CPrivateSendClientSession::SignFinalTransaction() and CPrivateSendServer::AddEntry() into CPrivateSendBaseSession::IsValidInOuts() * fix nit * Add missing return * Use CCoinsViewMemPool instead of doing it manually Co-authored-by: PastaPastaPasta <[email protected]> Co-authored-by: Alexander Block <[email protected]>
… txes (dashpay#3246) * Move PS mixing entry verification on masternodes into `AddEntry()` Also streamline logic a bit and drop unused/excessive parts. * Unify PS checks among masternodes and clients * No need to re-check outputs over and over again * No need to count, fail early if any output is missing * No need to look any further once we found the input we expected A tx with duplicate inputs would be considered invalid anyway and we also know there are no duplicates because we just verified the final tx above. Also drop an unused variable. * Unify LogPrint-s * Drop human-readable strings for unused PoolMessage-s * Apply suggestions from code review Co-Authored-By: PastaPastaPasta <[email protected]> * Re-introduce zero-fee checks * fix log * Move all txin/txout verification logic shared by CPrivateSendClientSession::SignFinalTransaction() and CPrivateSendServer::AddEntry() into CPrivateSendBaseSession::IsValidInOuts() * fix nit * Add missing return * Use CCoinsViewMemPool instead of doing it manually Co-authored-by: PastaPastaPasta <[email protected]> Co-authored-by: Alexander Block <[email protected]> Signed-off-by: cevap <[email protected]>
…has never been used. ac8a1d0 [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott) Pull request description: [BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block. Luke's implementation dashpay#936 did not include this flag, and it was later added to the help description in dashpay#3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description. If I am missing something or this is considered harmless - I can close the PR. Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44
… txes (dashpay#3246) * Move PS mixing entry verification on masternodes into `AddEntry()` Also streamline logic a bit and drop unused/excessive parts. * Unify PS checks among masternodes and clients * No need to re-check outputs over and over again * No need to count, fail early if any output is missing * No need to look any further once we found the input we expected A tx with duplicate inputs would be considered invalid anyway and we also know there are no duplicates because we just verified the final tx above. Also drop an unused variable. * Unify LogPrint-s * Drop human-readable strings for unused PoolMessage-s * Apply suggestions from code review Co-Authored-By: PastaPastaPasta <[email protected]> * Re-introduce zero-fee checks * fix log * Move all txin/txout verification logic shared by CPrivateSendClientSession::SignFinalTransaction() and CPrivateSendServer::AddEntry() into CPrivateSendBaseSession::IsValidInOuts() * fix nit * Add missing return * Use CCoinsViewMemPool instead of doing it manually Co-authored-by: PastaPastaPasta <[email protected]> Co-authored-by: Alexander Block <[email protected]> Signed-off-by: cevap <[email protected]>
… txes (dashpay#3246) * Move PS mixing entry verification on masternodes into `AddEntry()` Also streamline logic a bit and drop unused/excessive parts. * Unify PS checks among masternodes and clients * No need to re-check outputs over and over again * No need to count, fail early if any output is missing * No need to look any further once we found the input we expected A tx with duplicate inputs would be considered invalid anyway and we also know there are no duplicates because we just verified the final tx above. Also drop an unused variable. * Unify LogPrint-s * Drop human-readable strings for unused PoolMessage-s * Apply suggestions from code review Co-Authored-By: PastaPastaPasta <[email protected]> * Re-introduce zero-fee checks * fix log * Move all txin/txout verification logic shared by CPrivateSendClientSession::SignFinalTransaction() and CPrivateSendServer::AddEntry() into CPrivateSendBaseSession::IsValidInOuts() * fix nit * Add missing return * Use CCoinsViewMemPool instead of doing it manually Co-authored-by: PastaPastaPasta <[email protected]> Co-authored-by: Alexander Block <[email protected]> Signed-off-by: cevap <[email protected]>
Please see individual commits