backport: Merge bitcoin#20466,14604,15710,17458,18722,(partial) 17934 #5681
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijaydasmp
force-pushed
the
bp22_17
branch
2 times, most recently
from
6754a8f to
41a84a0
Compare
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_17
branch
3 times, most recently
from
c08b41a to
0a330c0
Compare
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_17
branch
2 times, most recently
from
fbef945 to
57e4387
Compare
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_17
branch
2 times, most recently
from
c4dcab8 to
704172f
Compare
|
Hello @UdjinM6 @knst, mac and Windows 64 build seem to have issue due to bitcoin#19455 changes, looking into it , meanwhile requesting suggestion |
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_17
branch
2 times, most recently
from
2fffd4d to
6f7a93c
Compare
knst
reviewed
Nov 17, 2023
There was a problem hiding this comment.
Please, check my comment for backport 17934
otherwise seems fine for me.
| @@ -118,7 +118,7 @@ def run_test(self): | |||
| block_hash = int(tip, 16) | |||
| self.send_block_request(block_hash, node0) | |||
| self.send_header_request(block_hash, node0) | |||
| node0.sync_with_ping() | |||
| node0.wait_for_block(block_hash, timeout=3) | |||
There was a problem hiding this comment.
note for reviewers: it is correct usage of block_hash indeed (see bitcoin#20047).
Backport is full because remaining changes will be in bitcoin#20047 (otherwise code won't run correctly)
| @@ -339,7 +339,8 @@ To build executables for ARM: | |||
| cd depends | |||
| make HOST=arm-linux-gnueabihf NO_QT=1 | |||
| cd .. | |||
| ./configure --prefix=$PWD/depends/arm-linux-gnueabihf --enable-reduce-exports LDFLAGS=-static-libstdc++ | |||
| ./autogen.sh | |||
| CONFIG_SITE=$PWD/depends/arm-linux-gnueabihf/share/config.site ./configure --enable-reduce-exports LDFLAGS=-static-libstdc++ | |||
There was a problem hiding this comment.
partial due to missing changes in doc/multiprocess.md
Can be done full only after bitcoin#18677
|
This pull request has conflicts, please rebase. |
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_17
branch
3 times, most recently
from
4ed54d3 to
d41b64a
Compare
vijaydasmp
changed the title
|
Hello @UdjinM6 @knst @ogabrielides @PastaPastaPasta , Please review |
|
Hello @UdjinM6 @knst @ogabrielides @PastaPastaPasta , requesting review |
|
Hello @PastaPastaPasta , Requesting review |
PastaPastaPasta
approved these changes
Dec 4, 2023
fad7be5 test: Fix intermittent p2p_finerprint issue (MarcoFalke) Pull request description: A single sync_with_ping can't be used to drop a block announcement, as the block might be sent *after* the ping has been responded to. Fix that by waiting for the block. ACKs for top commit: theStack: ACK fad7be5 Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
5531119 Added new test for future blocks reacceptance (sanket1729) 511a5af Fixed inconsistencies between code and comments (sanket1729) Pull request description: This Commit does 3 things: 1) Adds a test case for checking reacceptance a previously rejected block which was too far in the future. ~~2) clean up uses of rehash or calc_sha256 where it was not needed~~ 3) While constructing block 44, this commit makes the code consistent with the expected figure in the comment just above it by adding a transaction to the block. 4) Fix comment describing `sign_tx()` function ACKs for top commit: duncandean: reACK 5531119 brunoerg: reACK 5531119 Tree-SHA512: d40c72fcdbb0b2a0715adc58441eeea08147ee2ec5e371a4ccc824ebfdc6450698bd40aaeecb7ea7bfdb3cd1b264dd821b890276fff8b8d89b7225cdd9d6b546
7486e27 Tests: Unit test related to WalletDB ReadKeyValue (Bushstar) 32def8d Catch ios_base::failure specifically (Peter Bushnell) Pull request description: In bitcoin#2950 a hash of the pubkey and private was added to speed up key import, this was made backwards compatible by reading the hash in a try block with an ellipses catch all in case the hash was not present. CDataStream::read() specifically throws std::ios_base::failure, backwards compatibility expects only that error to be thrown, if something else gets thrown we should not be catching it. The change in this commit is to catch that exception only. If any other exception is thrown other than std::ios_base::failure it will be caught by the wider try block and an error written to the log and/or console. CDataStream::read() throwing std::ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/streams.h#L191 Wider catch statements that pick up all others exceptions other than ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L425 https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L430 ACKs for top commit: laanwj: Code review ACK 7486e27 Tree-SHA512: 5364bf935af8ec603bf5b8fef8c23b5cdaa4fe3506090cff988413221f2eaa99f7a91929afb42a35f8881ce2328744a0d32052da51ca0a5b2e65b6809e97f604
…s and filtering to occur within the struct 9adc2f8 Refactor OutputGroups to handle effective values, fees, and filtering (Andrew Chow) 7d07e86 Use real value when calculating OutputGroup value (Andrew Chow) Pull request description: Currently, the effective values and filtering for positive effective values is done outside of the OutputGroup. We should instead have functions in Outputgroup to do this and call those for each OutputGroup. So this PR does that. This makes future changes for effective values in coin selection much easier. ACKs for top commit: instagibbs: reACK 9adc2f8 fjahr: re-ACK 9adc2f8 meshcollider: Light code review ACK 9adc2f8 Tree-SHA512: 7445c94b7295b45bcd83a6f8a5c8f6961a89453fcc856335192d4b4a66aec7724513616b04e5111588ab208c89b311055399d6279cd9c4ce452aefb85f04b64a
…ble containers a92485b addrman: use unordered_map instead of map (Vasil Dimov) Pull request description: `CAddrMan` uses `std::map` internally even though it does not require that the map's elements are sorted. `std::map`'s access time is `O(log(map size))`. `std::unordered_map` is more suitable as it has a `O(1)` access time. This patch lowers the execution times of `CAddrMan`'s methods as follows (as per `src/bench/addrman.cpp`): ``` AddrMan::Add(): -3.5% AddrMan::GetAddr(): -76% AddrMan::Good(): -0.38% AddrMan::Select(): -45% ``` ACKs for top commit: jonatack: ACK a92485b achow101: ACK a92485b hebasto: re-ACK a92485b, only suggested changes and rebased since my [previous](bitcoin#18722 (review)) review. Tree-SHA512: d82959a00e6bd68a6c4c5a265dd08849e6602ac3231293b7a3a3b7bf82ab1d3ba77f8ca682919c15c5d601b13e468b8836fcf19595248116635f7a50d02ed603
…of --prefix option 223b1ba doc: Use CONFIG_SITE instead of --prefix (Hennadii Stepanov) Pull request description: The current examples of `--prefix=...` option usage to point `configure` script to appropriate `depends` directory is not [standard](https://www.gnu.org/prep/standards/html_node/Directory-Variables.html). This causes some [confusion](bitcoin#16691) and a bit of inconvenience. Consider a CentOS 7 32 bit system. Packages `libdb4-devel`, `libdb4-cxx-devel`, `miniupnpc-devel` and `zeromq-devel` are unavailable from repos. After recommended build with depends: ``` cd depends make cd .. ./autogen.sh ./configure --prefix=$PWD/depends/i686-pc-linux-gnu make ``` a user is unable to `make install` compiled binaries neither locally (to `~/.local`) nor system-wide (to `/usr/local`) as `--prefix` is set already. Meanwhile, the standard approach with using [`config.site`](https://www.gnu.org/software/automake/manual/html_node/config_002esite.html) files allows both possibilities: ``` cd depends make cd .. ./autogen.sh CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure --prefix ~/.local make make install ``` or ``` CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure make sudo make install # install to /usr/local ``` Moreover, this approach is used in [Gitian descriptors](https://github.com/bitcoin/bitcoin/tree/master/contrib/gitian-descriptors) already. ACKs for top commit: practicalswift: ACK 223b1ba: patch looks correct fanquake: ACK 223b1ba Tree-SHA512: 46d97924f0fc7e95ee4566737cf7c2ae805ca500e5c49af9aa99ecc3acede4b00329bc727a110aa1b62618dfbf5d1ca2234e736f16fbdf96d6ece5f821712f54
PastaPastaPasta
force-pushed
the
bp22_17
branch
from
d41b64a to
0181d2b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
bitcoin backports
What was done?
Backported bitcoin changes
How Has This Been Tested?
CI Run