Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ hebasto opened an issue: "`libbitcoinconsensus.a` is unusable"
(https://github.com/bitcoin/bitcoin/issues/28779)
On Ubuntu 22.04, in the master branch @ 0857f2935f90df9c3d303582e5b62a9c8dedd9d7:
```
$ ./autogen.sh
$ ./configure --without-daemon --without-gui --without-utils --disable-tests --disable-bench --disable-fuzz-binary --disable-shared --prefix=/
$ make
$ make DESTDIR=~/CONSENSUS install
$ g++ testconsensus.cpp -o testconsensus -I ~/CONSENSUS/include -L ~/CONSENSUS/lib -l:libbitcoinconsensus.a
/usr/bin/ld: /home/hebasto/CONSENSUS/lib/libbitcoinconsensus.a(libbitcoinconsensus_la-pubkey.o): in
...
💬 achow101 commented on pull request "test: add coverage to rpc_blockchain.py":
(https://github.com/bitcoin/bitcoin/pull/27852#issuecomment-1791405002)
ACK 376dc2cfb32806a8aa450589effe4d384e648398
🚀 achow101 merged a pull request: "test: add coverage to rpc_blockchain.py"
(https://github.com/bitcoin/bitcoin/pull/27852)
🤔 kevkevinpal reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1709331080)
ACK a5b6481b94580ef2cc2d9c45ac1c1959e79a82b2

added few small nits but feel free to ignore
💬 kevkevinpal commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379509252)
nit: Should we make some of these values into a variable instead of hardcoding the number?
💬 kevkevinpal commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379505261)
nit:
I just noticed `CENT/100` is used in this file for tx 5's fee aswell, not sure if it makes sense to make a common variable to use for both
💬 kevkevinpal commented on pull request "wallet: allowing walletpassphrase timeout to be MAX_SLEEP_TIME if set to -1":
(https://github.com/bitcoin/bitcoin/pull/28454#issuecomment-1791427196)
> rebased to [76b1c4c](https://github.com/bitcoin/bitcoin/pull/28454/commits/76b1c4ca6eea327328c7a0b035e6f2b1a2211e51) and since this [PR 28617](https://github.com/bitcoin/bitcoin/pull/28617) has been merged only the test-framework will be modified so I am not sure if this change is needed anymore.
>
> Thoughts @maflcko? I am happy to close this

closing this pull request
kevkevinpal closed a pull request: "wallet: allowing walletpassphrase timeout to be MAX_SLEEP_TIME if set to -1"
(https://github.com/bitcoin/bitcoin/pull/28454)
💬 achow101 commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#issuecomment-1791441941)
ACK bb91131d545d986aab81c4bb13676c4520169259
🚀 achow101 merged a pull request: "refactor: use string_view for passing string literals to Parse{Hash,Hex}"
(https://github.com/bitcoin/bitcoin/pull/28172)
💬 jonatack commented on issue "qa: Intermittent failure in `feature_fee_estimation.py`":
(https://github.com/bitcoin/bitcoin/issues/23165#issuecomment-1791553473)
This is probably resolved by the merge of https://github.com/bitcoin/bitcoin/pull/21161 and can be closed.
📝 bufo24 opened a pull request: "log: torcontrol opt checks"
(https://github.com/bitcoin/bitcoin/pull/28780)
Aims to fix #23589.
Host and port validation on torcontrol, same logic is being used on the other flags which need this validation as well.

This is my first PR for bitcoin core and came across this good first issue, continued on already existing code for this issue.

Took some inspiration from [this comment](https://github.com/bitcoin/bitcoin/pull/25136#pullrequestreview-1152866901)
🤔 TheCharlatan reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1710802872)
Concept ACK

lgtm, but can't really comment on the breadth of test cases.
💬 TheCharlatan commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1380467876)
In commit d525998b07be066c97a36536b58f0b98d816be39

Since you are changing the headers here, can you make them closer to what IWYU is suggesting?
💬 TheCharlatan commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1380802138)
Nit: The naming is confusing further down in the for loop, but I can't think of anything that's really better.
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380681598)
Upps yeah. This was intended to be -2.. fixing it.
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380719816)
> Is the intention here to mimic `NODE_NETWORK_LIMITED_MIN_BLOCKS` from the C++ source code, which is `288`?

Yeah, I used `MIN_BLOCKS_TO_KEEP` merely because it is already part of the test framework.

> `MIN_BLOCKS_TO_KEEP` is a different constant defined in both the C++ and Python code which happens to be also `288`. Should `NODE_NETWORK_LIMITED_MIN_BLOCKS` be defined in the Python code (equal to 288, but not necessary equal or tied to `MIN_BLOCKS_TO_KEEP`)?

If it helps readability, sur
...
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380733259)
> Is it possible to lookup something in the log or via RPC to cross check that the full node has made a decision not to request blocks from the pruned node?

Hmm, not really. We aren't logging any of the reasons behind a lack of a getdata request for a certain block from a certain peer (invalid tree, no segwit support, already in-flight). The approach would lead to excessive logging for no real gain.

What we could do, maybe in a follow-up?, to eliminate the unnecessary sleep time, is migrat
...
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1380702499)
> Should this not be `continue;`? I find `PeerManagerImpl::FindNextBlocks()` hard to follow. Are the entries in `vToFetch` sorted in ascending or descending order?

`vToFetch` is arranged in increasing order. Needs to stop once it reaches the last block the peer can provide, without continuing further.

`FindNextBlocks` begins from the last block in common with the peer. Then, during each iteration of the loop, it gets the block that is 128 blocks ahead and populates `vToFetch` by moving bac
...