🚀 achow101 merged a pull request: "guix: use osslsigncode 2.5"
(https://github.com/bitcoin/bitcoin/pull/27179)
(https://github.com/bitcoin/bitcoin/pull/27179)
💬 achow101 commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#issuecomment-1476594222)
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98
(https://github.com/bitcoin/bitcoin/pull/25939#issuecomment-1476594222)
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98
💬 TheCharlatan commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476594464)
Light code review ACK 5555a336019848d2e19e4533b3d2f34e438e6367
Confirmed that it indeed caches these additional packages. That said, I am not familiar with the build flow through the various scripts and cannot really comment if the code was moved to the correct place, or if conventions were followed.
> Would be good to make progress here.
Yes, I need to get a better scheduling workflow for PR's requiring interaction.
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476594464)
Light code review ACK 5555a336019848d2e19e4533b3d2f34e438e6367
Confirmed that it indeed caches these additional packages. That said, I am not familiar with the build flow through the various scripts and cannot really comment if the code was moved to the correct place, or if conventions were followed.
> Would be good to make progress here.
Yes, I need to get a better scheduling workflow for PR's requiring interaction.
💬 fanquake commented on pull request "build: Link `libbitcoinkernel` to `libbitcoin_util`":
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1476598738)
NACK. This is pretty literally the opposite of what we want to be doing with the kernel.
> And in master branch libbitcoinkernel_la_SOURCES and libbitcoin_util_a_SOURCES have almost the same source file lists.
The while point of having these files enumerated, is so that we know what is currently in the kernel, and can continue to (easily) prune deps, while refactoring. Hiding this away into `libbitcoin_util` only makes this harder/impossible to do, and you're just removing the current, exp
...
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1476598738)
NACK. This is pretty literally the opposite of what we want to be doing with the kernel.
> And in master branch libbitcoinkernel_la_SOURCES and libbitcoin_util_a_SOURCES have almost the same source file lists.
The while point of having these files enumerated, is so that we know what is currently in the kernel, and can continue to (easily) prune deps, while refactoring. Hiding this away into `libbitcoin_util` only makes this harder/impossible to do, and you're just removing the current, exp
...
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432253)
Fixed
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432253)
Fixed
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432364)
Done
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432364)
Done
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432481)
Done
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1142432481)
Done
💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476604650)
That said, it seems quite trivial to just change the default log level: https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b
So far that's not generating noise for me.
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476604650)
That said, it seems quite trivial to just change the default log level: https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b
So far that's not generating noise for me.
💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1476608456)
force push to: 93c70287a6434c6c665a211dc4dfbbd9c3db4083
- rebase on master
- fix failing unit test
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1476608456)
force push to: 93c70287a6434c6c665a211dc4dfbbd9c3db4083
- rebase on master
- fix failing unit test
💬 brunoerg commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142437445)
nit:
```suggestion
const std::string uri{"localhost:8080/rest/headers/someresource.json"};
```
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142437445)
nit:
```suggestion
const std::string uri{"localhost:8080/rest/headers/someresource.json"};
```
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142439278)
right
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142439278)
right
💬 TheCharlatan commented on pull request "build: Link `libbitcoinkernel` to `libbitcoin_util`":
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1476617574)
Concept NACK
> Isn't libbitcoin_util supposed to be a reusable build artifact?
Maybe, but linking in the entirety of the util library is not what the kernel project is currently trying to achieve. The goal is to prune as many files as possible out of the library. This also stated in stage 1 step 2 of the original kernel issue: https://github.com/bitcoin/bitcoin/issues/24303 . Once stage 2 is reached, there may be a discussion again of a shared build artifact containing the intersection of
...
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1476617574)
Concept NACK
> Isn't libbitcoin_util supposed to be a reusable build artifact?
Maybe, but linking in the entirety of the util library is not what the kernel project is currently trying to achieve. The goal is to prune as many files as possible out of the library. This also stated in stage 1 step 2 of the original kernel issue: https://github.com/bitcoin/bitcoin/issues/24303 . Once stage 2 is reached, there may be a discussion again of a shared build artifact containing the intersection of
...
✅ hebasto closed a pull request: "build: Link `libbitcoinkernel` to `libbitcoin_util`"
(https://github.com/bitcoin/bitcoin/pull/27285)
(https://github.com/bitcoin/bitcoin/pull/27285)
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142447667)
I wanted to make sure then to use the free up function for the request in the `HTTPRequest` destructor: `evhttp_request_free`. This was something I was doing while investigating the previous failure of the ASan tests and possible memory leaks. I'll remove it if you think unnecessary,
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142447667)
I wanted to make sure then to use the free up function for the request in the `HTTPRequest` destructor: `evhttp_request_free`. This was something I was doing while investigating the previous failure of the ASan tests and possible memory leaks. I'll remove it if you think unnecessary,
💬 jamesob commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476625515)
> That said, it seems quite trivial to just change the default log level: [Sjors@0ecb829](https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b)
>
> So far that's not generating noise for me.
I'm happy to do that and revert the revert if people will ACK.
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476625515)
> That said, it seems quite trivial to just change the default log level: [Sjors@0ecb829](https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b)
>
> So far that's not generating noise for me.
I'm happy to do that and revert the revert if people will ACK.
💬 MarcoFalke commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476626064)
> Yes, I need to get a better scheduling workflow for PR's requiring interaction.
I was planning to put it up for hi-prio, but I keep missing the meeting :sweat_smile:
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476626064)
> Yes, I need to get a better scheduling workflow for PR's requiring interaction.
I was planning to put it up for hi-prio, but I keep missing the meeting :sweat_smile:
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142449820)
ok, let me add some helper(s) there.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142449820)
ok, let me add some helper(s) there.
👍 sipa approved a pull request: "Add pool based memory resource"
(https://github.com/bitcoin/bitcoin/pull/25325)
ACK d87cb99bb37637e26a9e00b9f7de4bc6f44cb79d
(https://github.com/bitcoin/bitcoin/pull/25325)
ACK d87cb99bb37637e26a9e00b9f7de4bc6f44cb79d
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142466765)
Yeah, putting it back, before the compiler was complaining as in the previous approach we were modifying m_uri_parsed in this function.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142466765)
Yeah, putting it back, before the compiler was complaining as in the previous approach we were modifying m_uri_parsed in this function.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142467803)
Similar reason to the above comment on`GetURIParsed()` function, back again.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142467803)
Similar reason to the above comment on`GetURIParsed()` function, back again.