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

tx pool: Avoid passing redundant hash into addUnchecked (scripted-diff) #13792

Merged
merged 3 commits into from
Aug 29, 2018

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jul 29, 2018

Several years ago the transaction hash was not cached. For optimization the hash was instead passed into addUnchecked to avoid re-calculating it. See f77654a

Passing in the hash is now redundant and the argument can safely be removed.

@maflcko maflcko force-pushed the Mf1808-constTxPoolEntries branch 3 times, most recently from 5bbe638 to 8c9d589 Compare July 29, 2018 01:31
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 29, 2018

Note to reviewers: This pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member

fanquake commented Jul 29, 2018

Looks like the scripted-diff is failing because of the other whitespace/formatting changes also included in 8c9d589.

@maflcko maflcko force-pushed the Mf1808-constTxPoolEntries branch 2 times, most recently from 21a6366 to 376e27b Compare July 30, 2018 13:07
-BEGIN VERIFY SCRIPT-
git grep -l addUnchecked | xargs sed --regexp-extended -i -e 's/addUnchecked\([^)][^,]+,\s*/addUnchecked(/g'
-END VERIFY SCRIPT-
@maflcko maflcko force-pushed the Mf1808-constTxPoolEntries branch 2 times, most recently from 0e97ce7 to fa58777 Compare July 30, 2018 20:32
@maflcko maflcko changed the title tx pool: Avoid passing redundant hash into addUnchecked tx pool: Avoid passing redundant hash into addUnchecked (scripted-diff) Jul 30, 2018
@maflcko
Copy link
Member Author

maflcko commented Jul 30, 2018

This is split off of #13804, but not required. So if people hate this kind of refactoring, I am happy to drop it.

@bitcoin bitcoin deleted a comment from DrahtBot Jul 30, 2018
@promag
Copy link
Member

promag commented Jul 31, 2018

utACK fa58777. I like the change, passing entry is now enough. Could update OP now that #13786 is merged.

@practicalswift
Copy link
Contributor

Concept ACK

@Empact
Copy link
Member

Empact commented Aug 2, 2018

utACK fa58777

@laanwj
Copy link
Member

laanwj commented Aug 29, 2018

utACK fe5c497
removing the redundancy is good because it avoids a potential bug when passing in the wrong hash

@laanwj laanwj merged commit fa58777 into bitcoin:master Aug 29, 2018
laanwj added a commit that referenced this pull request Aug 29, 2018
… (scripted-diff)

fa58777 scripted-diff: Remove unused first argument to addUnchecked (MarcoFalke)
fe5c497 tx pool: Use the entry's hash instead of the one passed to addUnchecked (MarcoFalke)
ddd395f Mark CTxMemPoolEntry members that should not be modified const (MarcoFalke)

Pull request description:

  Several years ago the transaction hash was not cached. For optimization the hash was instead passed into `addUnchecked` to avoid re-calculating it. See f77654a

  Passing in the hash is now redundant and the argument can safely be removed.

Tree-SHA512: 0206b65c7a014295f67574120e8c5397bf1b1bd70c918ae1360ab093676f7f89a6f084fd2c7000a141baebfe63fe6f515559e38c4ac71810ba64f949f9c0467f
@maflcko maflcko deleted the Mf1808-constTxPoolEntries branch August 29, 2018 14:39
domob1812 added a commit to domob1812/namecoin-core that referenced this pull request Sep 3, 2018
Update auxpow_tests.cpp for the upstream refactoring in
bitcoin/bitcoin#13792.
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jun 30, 2021
…checked (scripted-diff)

fa58777 scripted-diff: Remove unused first argument to addUnchecked (MarcoFalke)
fe5c497 tx pool: Use the entry's hash instead of the one passed to addUnchecked (MarcoFalke)
ddd395f Mark CTxMemPoolEntry members that should not be modified const (MarcoFalke)

Pull request description:

  Several years ago the transaction hash was not cached. For optimization the hash was instead passed into `addUnchecked` to avoid re-calculating it. See f77654a

  Passing in the hash is now redundant and the argument can safely be removed.

Tree-SHA512: 0206b65c7a014295f67574120e8c5397bf1b1bd70c918ae1360ab093676f7f89a6f084fd2c7000a141baebfe63fe6f515559e38c4ac71810ba64f949f9c0467f

# Conflicts:
#	src/bench/mempool_eviction.cpp
#	src/test/mempool_tests.cpp
#	src/test/miner_tests.cpp
#	src/txmempool.cpp
#	src/txmempool.h
#	src/validation.cpp
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 18, 2021
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 18, 2021
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 18, 2021
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 19, 2021
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 19, 2021
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants