Remove redundant parameter fCheckDuplicateInputs from CheckTransaction(...) #14258
Conversation
| No more conflicts as of last run. |
|
utACK 0172ba4 |
|
utACK 0172ba4 |
|
utACK 0172ba4 |
|
utACK 0172ba4. |
|
NACK we'll probably re-introduce the optimization at some point, let's avoid the code churn.
…On September 18, 2018 3:42:40 PM UTC, practicalswift ***@***.***> wrote:
Remove redundant parameter `fCheckDuplicateInputs` from
`CheckTransaction(...)`.
Diff without whitespace noise:
[`?w=1`](https://github.com/bitcoin/bitcoin/pull/14258/files?w=1)
You can view, comment on, or merge this pull request online at:
#14258
-- Commit Summary --
* Remove redundant parameter fCheckDuplicateInputs from
CheckTransaction(...)
-- File Changes --
M src/consensus/tx_verify.cpp (15)
M src/consensus/tx_verify.h (2)
M src/validation.cpp (2)
-- Patch Links --
https://github.com/bitcoin/bitcoin/pull/14258.patch
https://github.com/bitcoin/bitcoin/pull/14258.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#14258
|
|
IMO leaving unused code that can lead to bugs is worst than code churn. This can be reverted along with proper review/testing once and if necessary. |
|
I am in agreement with @promag and was about to say similar. |
|
Re-opened since consensus opinion seems to closer to be ACK side than the NACK side :-) |
|
@practicalswift to avoid noise please gather more comments before changing PR status. |
|
utACK 0172ba4. It is safer to remove this unused code now. Any future optimizations should be written in a different way that is more clear and not easily misunderstood as benign. |
|
utACK 0172ba4 |
|
NACK right now. Let's please take some time to study the reasons that caused the issue first before we touch this code further. |
|
I agree with @sipa, cleaning up this code is not a priority now. |
11 participants
Remove redundant parameter
fCheckDuplicateInputsfromCheckTransaction(...).Diff without whitespace noise:
?w=1