💬 fanquake commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517926243)
Put on top of #27483.
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517926243)
Put on top of #27483.
📝 fanquake converted_to_draft a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507)
Type stubs are now available as of 0.13.0.
See https://github.com/lief-project/LIEF/issues/650.
(https://github.com/bitcoin/bitcoin/pull/27507)
Type stubs are now available as of 0.13.0.
See https://github.com/lief-project/LIEF/issues/650.
💬 batriskaweb3 commented on issue "Compiling a bitcoin core version that accepts transactions over 100vkb":
(https://github.com/bitcoin/bitcoin/issues/27490#issuecomment-1517945110)
this is pretty important for the entire ordinals community as it will allow them to inscribe ordinals bigger than 400kb on the blockchain
(https://github.com/bitcoin/bitcoin/issues/27490#issuecomment-1517945110)
this is pretty important for the entire ordinals community as it will allow them to inscribe ordinals bigger than 400kb on the blockchain
🤔 dergoegge reviewed a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1395938681)
lgtm once the last `gArgs` access is removed.
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1395938681)
lgtm once the last `gArgs` access is removed.
💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173878733)
```suggestion
return FlatFileSeq(m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kb */ : BLOCKFILE_CHUNK_SIZE);
```
The "util/system.h" include can almost be removed after this. `ScheduleBatchPriority()` is the only function that is still used.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173878733)
```suggestion
return FlatFileSeq(m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kb */ : BLOCKFILE_CHUNK_SIZE);
```
The "util/system.h" include can almost be removed after this. `ScheduleBatchPriority()` is the only function that is still used.
💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173885220)
nit: Seems unnecessary to pass the entire `NodeContext` when you only need the blockman.
Feel free to ignore, but would be interested in why you chose to do it this way.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173885220)
nit: Seems unnecessary to pass the entire `NodeContext` when you only need the blockman.
Feel free to ignore, but would be interested in why you chose to do it this way.
🤔 pablomartin4btc reviewed a pull request: "wallet: finish addressbook encapsulation"
(https://github.com/bitcoin/bitcoin/pull/26836#pullrequestreview-1395963136)
re-ACK 8fa0db1d487d76e3fa5ace0bd4d9a8b6bf8dbde1
(https://github.com/bitcoin/bitcoin/pull/26836#pullrequestreview-1395963136)
re-ACK 8fa0db1d487d76e3fa5ace0bd4d9a8b6bf8dbde1
💬 pinheadmz commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#discussion_r1173902633)
Yeah I think thats just to decode numeric addresses like "100.200.30.40" etc. It looked to me like if an actual DNS request is necessary it has its own methods to make reqests directly to the nameservers provided by the platform? When things get "serious":
https://github.com/libevent/libevent/blob/master/evdns.c#L5623
(https://github.com/bitcoin/bitcoin/pull/27505#discussion_r1173902633)
Yeah I think thats just to decode numeric addresses like "100.200.30.40" etc. It looked to me like if an actual DNS request is necessary it has its own methods to make reqests directly to the nameservers provided by the platform? When things get "serious":
https://github.com/libevent/libevent/blob/master/evdns.c#L5623
💬 mzumsande commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1518004494)
Concept ACK. Not sure if this should be the default or an optional mode (in some cases, a higher chance of transmission may be more important than privacy).
It might also be helpful to combine this with some heuristics that the tx percolated through the network:
For example, require that 2 of your regular full outbound connections (that you didn't send it to ) have inv'ed the tx back to you - otherwise try again after some time.
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1518004494)
Concept ACK. Not sure if this should be the default or an optional mode (in some cases, a higher chance of transmission may be more important than privacy).
It might also be helpful to combine this with some heuristics that the tx percolated through the network:
For example, require that 2 of your regular full outbound connections (that you didn't send it to ) have inv'ed the tx back to you - otherwise try again after some time.
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173921394)
The blockman and chainstate instances may be deleted during the lifetime of the zmq instance similarly as described in mzsumsande's [comment](https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1160180267) when iterating the for loop. If we pass a reference to the `NodeContext` instead, we have better guarantees that its blockman reference points to an actual object.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173921394)
The blockman and chainstate instances may be deleted during the lifetime of the zmq instance similarly as described in mzsumsande's [comment](https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1160180267) when iterating the for loop. If we pass a reference to the `NodeContext` instead, we have better guarantees that its blockman reference points to an actual object.
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173922264)
Good catch, thank you! Will also remove the `<args.h>` include.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173922264)
Good catch, thank you! Will also remove the `<args.h>` include.
🤔 brunoerg reviewed a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1396013005)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1396013005)
Concept ACK
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518024541)
Thank you for the review @dergoegge,
Updated 1a7d73d30e312e862be4560601302e79725b1bc7 -> ed3159e0eb105d9f46659bd8f8b2db27d94841de ([removeBlockstorageArgs_14](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_14) -> [removeBlockstorageArgs_15](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_15), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_14..removeBlockstorageArgs_15))
* Addressed @dergoegge's [comment](https://gith
...
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518024541)
Thank you for the review @dergoegge,
Updated 1a7d73d30e312e862be4560601302e79725b1bc7 -> ed3159e0eb105d9f46659bd8f8b2db27d94841de ([removeBlockstorageArgs_14](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_14) -> [removeBlockstorageArgs_15](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_15), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_14..removeBlockstorageArgs_15))
* Addressed @dergoegge's [comment](https://gith
...
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1173930525)
Done on the latest push
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1173930525)
Done on the latest push
💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173938033)
Is that actually possible? Seems like all the validation interfaces including the zmq one are unregistered before chainman is destroyed?
So it can't be holding a reference to blockman that is actually destroyed, see:
https://github.com/bitcoin/bitcoin/blob/f3f5c9712670fc2c9fb3b57580146aa248a5d36f/src/init.cpp#L327-L343
Or are you just saying that having a reference to the node context has better lifetime guarantees because basically nothing outlives the node context?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173938033)
Is that actually possible? Seems like all the validation interfaces including the zmq one are unregistered before chainman is destroyed?
So it can't be holding a reference to blockman that is actually destroyed, see:
https://github.com/bitcoin/bitcoin/blob/f3f5c9712670fc2c9fb3b57580146aa248a5d36f/src/init.cpp#L327-L343
Or are you just saying that having a reference to the node context has better lifetime guarantees because basically nothing outlives the node context?
💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518037638)
Code review ACK bab12a6a029bed86ddcc15d42a236a855e277e74
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518037638)
Code review ACK bab12a6a029bed86ddcc15d42a236a855e277e74
💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518046601)
Code review ACK ed3159e0eb105d9f46659bd8f8b2db27d94841de
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518046601)
Code review ACK ed3159e0eb105d9f46659bd8f8b2db27d94841de
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173954405)
> Is that actually possible?
Likely that I am missing something in the logic. The way I see it each iteration of the for loop creates and destroys the `chainman` by calling `std::make_unique` again, while the `Shutdown` code is not called during each iteration.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173954405)
> Is that actually possible?
Likely that I am missing something in the logic. The way I see it each iteration of the for loop creates and destroys the `chainman` by calling `std::make_unique` again, while the `Shutdown` code is not called during each iteration.
👍 TheCharlatan approved a pull request: "build: use latest config.{guess,sub} in depends"
(https://github.com/bitcoin/bitcoin/pull/27508#pullrequestreview-1396060103)
ACK 4a3f1db4ea5f90277cf7f57c051a2285e8b42468
I also checked the content from
```
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' -O config.guess
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.sub;hb=HEAD' -O config.sub
```
Guix builds:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
fef9152d593aa1cb3243f37b8c069ef863f4e270962e17918cfa8
...
(https://github.com/bitcoin/bitcoin/pull/27508#pullrequestreview-1396060103)
ACK 4a3f1db4ea5f90277cf7f57c051a2285e8b42468
I also checked the content from
```
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' -O config.guess
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.sub;hb=HEAD' -O config.sub
```
Guix builds:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
fef9152d593aa1cb3243f37b8c069ef863f4e270962e17918cfa8
...
💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808)
> Seems kind of weird to change the CLI to use a "test-only" RPC in any case
@fanquake, @amitiuttarwar `-generate` CLI currently [uses](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/bitcoin-cli.cpp#L1178) a [hidden RPC](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/rpc/mining.cpp#L1055) (`generatetoaddress`) too.
i don't think a normal user would be interested in the new/tried table breakdown of addresses. so hop
...
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808)
> Seems kind of weird to change the CLI to use a "test-only" RPC in any case
@fanquake, @amitiuttarwar `-generate` CLI currently [uses](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/bitcoin-cli.cpp#L1178) a [hidden RPC](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/rpc/mining.cpp#L1055) (`generatetoaddress`) too.
i don't think a normal user would be interested in the new/tried table breakdown of addresses. so hop
...