💬 sdaftuar commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1588067475)
Inspired by your approach, I thought it might be helpful to just add support for setting a target feerate on a transaction in miniwallet, something like this?
```diff
diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..db6aa78167 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -533,13 +533,25 @@ class MempoolAcceptV3(BitcoinTestFramework):
tx_unrelated_replacee = self.wallet.s
...
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1588067475)
Inspired by your approach, I thought it might be helpful to just add support for setting a target feerate on a transaction in miniwallet, something like this?
```diff
diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..db6aa78167 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -533,13 +533,25 @@ class MempoolAcceptV3(BitcoinTestFramework):
tx_unrelated_replacee = self.wallet.s
...
💬 murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2091226696)
> 6 hours seems overkill to me. I'd think 3 hours would suffice, since it would mean that at worst someone would be able to generate a block 1 hour after the previous block if they also warped the time ahead by the max 2 hours.
Three hours seems too long to me. That would mean that the first use of the low-difficulty loophole would require one hour of wait (by also setting your clock two hours to the future), but any subsequent block would be delayed by three hours. It would be trivial for an
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2091226696)
> 6 hours seems overkill to me. I'd think 3 hours would suffice, since it would mean that at worst someone would be able to generate a block 1 hour after the previous block if they also warped the time ahead by the max 2 hours.
Three hours seems too long to me. That would mean that the first use of the low-difficulty loophole would require one hour of wait (by also setting your clock two hours to the future), but any subsequent block would be delayed by three hours. It would be trivial for an
...
💬 sipsorcery commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588166620)
This check will fail on Windows. I'm guessing that's expected but just checking?
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588166620)
This check will fail on Windows. I'm guessing that's expected but just checking?
💬 vasild commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091280337)
This is indeed some problem in FreeBSD's libc which I reported upstream with a minimal, reproducable test case:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701
Is anybody interested in reviewing the patch I posted above if I PR it:
https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2079776241
reworked to return `std::string_view` to the callers, but still storing the thread name in `char []`? That would work around the FreeBSD weird message printout because the `thread
...
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091280337)
This is indeed some problem in FreeBSD's libc which I reported upstream with a minimal, reproducable test case:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701
Is anybody interested in reviewing the patch I posted above if I PR it:
https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2079776241
reworked to return `std::string_view` to the callers, but still storing the thread name in `char []`? That would work around the FreeBSD weird message printout because the `thread
...
💬 vasild commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-2091285732)
ping @maflcko
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-2091285732)
ping @maflcko
💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588204550)
The goal is to detect that binaries, which are cross-compiled for Windows, are running in Wine.
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588204550)
The goal is to detect that binaries, which are cross-compiled for Windows, are running in Wine.
💬 laanwj commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091354481)
> reworked to return std::string_view to the callers, but still storing the thread name in a thread_local char []?
Yes. i think (for this one very rare exception) it's acceptable to store a string in a fixed-size buffer. To not need a destructor and heap deallocation when a thread goes away, works around a large part of the complexity of handling thread-local data.
And making it use `string_view` throughout the changed functions instead of `char*` is a good idea, a lot less ugly.
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091354481)
> reworked to return std::string_view to the callers, but still storing the thread name in a thread_local char []?
Yes. i think (for this one very rare exception) it's acceptable to store a string in a fixed-size buffer. To not need a destructor and heap deallocation when a thread goes away, works around a large part of the complexity of handling thread-local data.
And making it use `string_view` throughout the changed functions instead of `char*` is a good idea, a lot less ugly.
💬 jonatack commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091356712)
> maybe just replace 520 with MAX_SCRIPT_ELEMENT_SIZE and nothing else?
Thanks @instagibbs, done.
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091356712)
> maybe just replace 520 with MAX_SCRIPT_ELEMENT_SIZE and nothing else?
Thanks @instagibbs, done.
💬 laanwj commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2091363954)
Yes, adding an example would be useful.
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2091363954)
Yes, adding an example would be useful.
💬 instagibbs commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091366440)
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091366440)
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860
💬 sipsorcery commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588252435)
Does that mean this PR is intended to be a fix for Windows or for wine?
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588252435)
Does that mean this PR is intended to be a fix for Windows or for wine?
🤔 mzumsande reviewed a pull request: "net: don't lock cs_main while reading blocks in net processing"
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-2036711916)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-2036711916)
Concept ACK
💬 mzumsande commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121)
As written somewhere above, we never prune blocks > 288 blocks from the tip, and if pruning, we should have disconnected any peer requesting older blocks - so it should be impossible to trigger this log even with pruning.
On the other hand, it could still be hit while not pruning due to disk problems - e.g. helping other peers with IBD. Searching for this assert gives me multiple matches, e.g.
* #6263
* #5668
* #12230
* https://bitcoin.stackexchange.com/questions/113218/how-to-fix-the-bitc
...
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121)
As written somewhere above, we never prune blocks > 288 blocks from the tip, and if pruning, we should have disconnected any peer requesting older blocks - so it should be impossible to trigger this log even with pruning.
On the other hand, it could still be hit while not pruning due to disk problems - e.g. helping other peers with IBD. Searching for this assert gives me multiple matches, e.g.
* #6263
* #5668
* #12230
* https://bitcoin.stackexchange.com/questions/113218/how-to-fix-the-bitc
...
💬 sipsorcery commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#issuecomment-2091446475)
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203
(https://github.com/bitcoin/bitcoin/pull/30017#issuecomment-2091446475)
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203
💬 willcl-ark commented on pull request "Fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#issuecomment-2091482504)
Sorry, I've addressed the linter now.
(https://github.com/bitcoin-core/gui/pull/819#issuecomment-2091482504)
Sorry, I've addressed the linter now.
💬 achow101 commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#issuecomment-2091483193)
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62
(https://github.com/bitcoin/bitcoin/pull/29961#issuecomment-2091483193)
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62
💬 andrewtoth commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588303739)
> so it should be impossible to trigger this log even with pruning
Since we have a 2 block buffer, peers can request up to 290 blocks. So one way this can be triggered - a peer requests block 290 and we get the filepos, we release the lock, `pruneblockchain` RPC is called, rpc thread acquires the lock and block 290 is pruned before we can acquire the fd, read now fails.
However, `pruneblockchain` has a 2 hour buffer, so there must also be a long window of no new blocks.
I don't think this i
...
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588303739)
> so it should be impossible to trigger this log even with pruning
Since we have a 2 block buffer, peers can request up to 290 blocks. So one way this can be triggered - a peer requests block 290 and we get the filepos, we release the lock, `pruneblockchain` RPC is called, rpc thread acquires the lock and block 290 is pruned before we can acquire the fd, read now fails.
However, `pruneblockchain` has a 2 hour buffer, so there must also be a long window of no new blocks.
I don't think this i
...
💬 willcl-ark commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2091500828)
I've been using this in a side-project and noticed some unintended behaviour, which could be my system; if I `make distclean` (removing the generated files) and then `make -j16` the build fails as it can't find the built artefacts. Running plain `make` works fine every time, and _sometimes_ a low job number works too.
The Makefile [appears](https://github.com/ryanofsky/bitcoin/blob/56ef459b573461087fbe4f39d9b0a7108936335f/src/Makefile.am#L1083-L1098) (to me) to have the dependencies listed co
...
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2091500828)
I've been using this in a side-project and noticed some unintended behaviour, which could be my system; if I `make distclean` (removing the generated files) and then `make -j16` the build fails as it can't find the built artefacts. Running plain `make` works fine every time, and _sometimes_ a low job number works too.
The Makefile [appears](https://github.com/ryanofsky/bitcoin/blob/56ef459b573461087fbe4f39d9b0a7108936335f/src/Makefile.am#L1083-L1098) (to me) to have the dependencies listed co
...
💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588314813)
For both. Wine is used to run test suite on Linux after cross-compiling for Windows. This need root as Wine and Windows runtime produce different error messages: https://github.com/bitcoin/bitcoin/blob/db73ac8c517f91ed5175b3635eb37ed56852ff91/src/test/system_tests.cpp#L80
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588314813)
For both. Wine is used to run test suite on Linux after cross-compiling for Windows. This need root as Wine and Windows runtime produce different error messages: https://github.com/bitcoin/bitcoin/blob/db73ac8c517f91ed5175b3635eb37ed56852ff91/src/test/system_tests.cpp#L80
💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588317380)
FWIW, the same behaviour was before https://github.com/bitcoin/bitcoin/pull/29489.
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588317380)
FWIW, the same behaviour was before https://github.com/bitcoin/bitcoin/pull/29489.