Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 ismaelsadeeq approved a pull request: "test: fix inconsistency in fundrawtransaction weight limits test"
(https://github.com/bitcoin/bitcoin/pull/30353#pullrequestreview-2171797056)
Code review and Tested ACK 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668

I was able to recreate the failure with the diff in the OP. I also decoded the transaction and confirmed that it's selecting the change output of the parent transaction with a value of 2.79949465 BTC. That's why the test was not failing.

The commit 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668 fixes this issue.

> This is not a problem for `fundrawtransaction()`

Yes, it is not. The call will succeed, but when attempting to
...
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1673957302)
That seems incorrect. `ActivateBestchain` exits before `ActiveTipChange` if it realizes there is nothing to do, e.g. if `invalidateblock` only causes the tip to change from block n to block n-1.

https://github.com/bitcoin/bitcoin/blob/9b480f7a25a737c9c4ebc33401e94d66c2da9ec3/src/validation.cpp#L3505-L3508
🤔 ismaelsadeeq reviewed a pull request: "remove truc_policy from libbitcoin_common_a_SOURCES"
(https://github.com/bitcoin/bitcoin/pull/30427#pullrequestreview-2171804272)
ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2222894099)
Addressed linter issues and fixed deadlock regression in 02f1f86b6b6d46d8ab5e17d5d824b0c1bba5d789 -> 00201802a19124cc8eb671b7f9311326f99f999d.
💬 Eunovo commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2222925877)
> You can squash some commits.

Thanks @brunoerg. Done!
💬 Eunovo commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674013463)
@glozow Thanks for the clarification. I see it now. Any reason why we don't fire the `ActiveTipChange` here instead?
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674034705)
What do you mean by "here"?
💬 glozow commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1672095708)
probably `FeeFrac` would be better? `CFeeRate` loses precision and can be inferred from the fee and size
💬 Eunovo commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674043326)
By "here", I mean in https://github.com/bitcoin/bitcoin/blob/9b480f7a25a737c9c4ebc33401e94d66c2da9ec3/src/validation.cpp#L3505-L3508
🤔 hebasto reviewed a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423#pullrequestreview-2171924002)
Concept ACK.

nit: Spaces after commas and around `+` can used more consistently for better readability.
💬 hebasto commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674044249)
This check fails:
```
FAIL: test_PE (__main__.TestSecurityChecks)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/distsrc-base/distsrc-ea096063c384-x86_64-w64-mingw32/./contrib/devtools/test-security-check.py", line 88, in test_PE
self.assertEqual(call_security_check(cc, source, executable, pass_flags + ['-Wl,--disable-reloc-section']), (1, executable+': failed RELOC_SECTION'))
AssertionError: Tuples differ: (1, 'test1
...
💬 hebasto commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674030590)
Is the `-pie` flag supported here?

From the `ld` linker [docs](https://sourceware.org/binutils/docs/ld.html#index-_002dpie):
> This is currently only supported on ELF platforms.
💬 fanquake commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674050332)
Those look like the wrong docs:
```bash
x86_64-w64-mingw32-ld --help | grep pie
-pie, --pic-executable Create a position independent executable
-no-pie Create a position dependent executable (default)
```
💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#discussion_r1674079452)
Ok thanks again, I'll make sure it gets corrected at that time
💬 fanquake commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674084480)
This has turned up what looks like a bug in LIEF: https://github.com/lief-project/LIEF/issues/1076. Will re-enable the test later.
🤔 glozow reviewed a pull request: "#28984 package rbf followups"
(https://github.com/bitcoin/bitcoin/pull/30295#pullrequestreview-2172049102)
ACK 3f00aae1409
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1674109620)
We need the vector since order has to be preserved, and since the number of elements should be small, I don't think the performance impact is significant. Also, `std::find` is `O(n)`.
📝 maflcko opened a pull request: " log: LogError with FlatFilePos in UndoReadFromDisk "
(https://github.com/bitcoin/bitcoin/pull/30428)
These errors should never happen in normal operation. If they do,
knowing the `FlatFilePos` may be useful to determine if data corruption
happened. Also, handle the error `pos.IsNull()` as part of `OpenUndoFile`,
because it may as well have happened due to data corruption.

This mirrors the `LogError` behavior from `ReadBlockFromDisk`.

Also, two other fixup commits in this module.
💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#issuecomment-2223121276)
cc @murchandamus, incorporates some of your test improvement suggestions https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643074768
💬 TheBlueMatt commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2223137778)
> I sympathize with the argument that the harder we make things for the user, the less likely they are to run the software. I strongly disagree, however, with the conclusion that the best solution then is to put everything into a single binary for the user. All that matters for the user is that it Just Works ™️ and there are plenty of ways we can achieve this regardless of the technical design.

Right, I don't at think anyone was making the argument that it *has* to be in the same process, but
...