💬 vasild commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2202968468)
Ok, in the case with `python3 -m http.server 60000` the reply is `<!DOCTYPE HTML>`. In the CI the reply is `HELLO VERSION MIN=3.1 MAX=3.1`, same as the request. So some "echo tcp server" is running on port 60000!?
I wonder what is a sure way to find an address and port that will fail the connection quickly? Maybe pick one of the "reserved" ports from https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers, e.g. `127.0.0.1:250`? Or use the P2P port of node0 (`p2p_port(0)`), `bitcoind` i
...
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2202968468)
Ok, in the case with `python3 -m http.server 60000` the reply is `<!DOCTYPE HTML>`. In the CI the reply is `HELLO VERSION MIN=3.1 MAX=3.1`, same as the request. So some "echo tcp server" is running on port 60000!?
I wonder what is a sure way to find an address and port that will fail the connection quickly? Maybe pick one of the "reserved" ports from https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers, e.g. `127.0.0.1:250`? Or use the P2P port of node0 (`p2p_port(0)`), `bitcoind` i
...
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2202983850)
@cbergqvist,
> The current solution means most test nodes in the functional test suite will now bind to an onion port even though the vast majority are not testing Tor
Only tests that use `bind_to_localhost_only` will do. We have 5 such tests.
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2202983850)
@cbergqvist,
> The current solution means most test nodes in the functional test suite will now bind to an onion port even though the vast majority are not testing Tor
Only tests that use `bind_to_localhost_only` will do. We have 5 such tests.
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1662415420)
I forgot. Applied it now.
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1662415420)
I forgot. Applied it now.
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2203021387)
`ffe9b27498...bca346a970`: take minor suggestion which I forgot earlier: use `expected_services` instead of `self.expected[i][1]` in the test.
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2203021387)
`ffe9b27498...bca346a970`: take minor suggestion which I forgot earlier: use `expected_services` instead of `self.expected[i][1]` in the test.
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-2203050991)
Rebased and updated 34970bde23dd87fc0fea5a1970880761f7184774 -> 1a1e2dac0c657c3ee96c7faf9594aa0bed770702 ([kernelInternalLib_12](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_12) -> [kernelInternalLib_13](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_13))
#29015 got merged in the meantime and there are some implications for this pull request. We decided that the util library may contain a superset of the utilities that the kernel strictly requires.
For th
...
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-2203050991)
Rebased and updated 34970bde23dd87fc0fea5a1970880761f7184774 -> 1a1e2dac0c657c3ee96c7faf9594aa0bed770702 ([kernelInternalLib_12](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_12) -> [kernelInternalLib_13](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_13))
#29015 got merged in the meantime and there are some implications for this pull request. We decided that the util library may contain a superset of the utilities that the kernel strictly requires.
For th
...
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203056850)
Thank you for the review @glozow,
Rebased 064a401e10f2b2da8f31f682929431d06c671d93 -> 732879b99565aa21c6cab8c754e67cf1d9425854 ([noGlobalScriptCache_9](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_9) -> [noGlobalScriptCache_10](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_9..noGlobalScriptCache_10))
* Fixed conflict with #30344
Updated 732879b99565aa21c6cab8c754e67c
...
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203056850)
Thank you for the review @glozow,
Rebased 064a401e10f2b2da8f31f682929431d06c671d93 -> 732879b99565aa21c6cab8c754e67cf1d9425854 ([noGlobalScriptCache_9](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_9) -> [noGlobalScriptCache_10](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_9..noGlobalScriptCache_10))
* Fixed conflict with #30344
Updated 732879b99565aa21c6cab8c754e67c
...
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1662443420)
Decided to just initialize it statically. If someone wants to fuzz the constructor interface, I think that could go into a follow-up.
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1662443420)
Decided to just initialize it statically. If someone wants to fuzz the constructor interface, I think that could go into a follow-up.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2203060643)
`11be9f4103...ea89a6e368`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2203060643)
`11be9f4103...ea89a6e368`: rebase due to conflicts
💬 brunoerg commented on pull request "fuzz: fix ciphertext size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203084854)
Rebased to fix CI
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203084854)
Rebased to fix CI
👍 TheCharlatan approved a pull request: "util: Use SteadyClock in RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/30372#pullrequestreview-2153736635)
ACK fa360b047fc578fd855b8f7420d84dc967884f4a
(https://github.com/bitcoin/bitcoin/pull/30372#pullrequestreview-2153736635)
ACK fa360b047fc578fd855b8f7420d84dc967884f4a
👍 stickies-v approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2153906841)
re-ACK 45e0c96e81b5f0c364af717b7cb2d9bc094372de
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2153906841)
re-ACK 45e0c96e81b5f0c364af717b7cb2d9bc094372de
👍 pinheadmz approved a pull request: "Fix cases of calls to `FillPSBT` errantly returning `complete=true`"
(https://github.com/bitcoin/bitcoin/pull/30357#pullrequestreview-2153928626)
ACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
Reviewed code and confirmed test fails without patch. Approach is great, one question below about how much farther to take this approach...
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaECVgACgkQ5+KYS2KJ
yTq5vw//QNiwjThHf65qtM+57G6YDPWQ3zVu2s278T0aZU5ND62Hu4A9/DHqD
...
(https://github.com/bitcoin/bitcoin/pull/30357#pullrequestreview-2153928626)
ACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
Reviewed code and confirmed test fails without patch. Approach is great, one question below about how much farther to take this approach...
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaECVgACgkQ5+KYS2KJ
yTq5vw//QNiwjThHf65qtM+57G6YDPWQ3zVu2s278T0aZU5ND62Hu4A9/DHqD
...
💬 pinheadmz commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1662588243)
This makes me wonder if `finalize=True` should result in `"complete": True`. It's another case of using `PSBTInputSigned()` instead of `PSBTInputSignedAndVerified()`. We could update this code below to automatically replace / update invalid signatures.
https://github.com/bitcoin/bitcoin/blob/d2c8d161b46bd62256a17abd086d8ae138c043c3/src/wallet/wallet.cpp#L2181-L2194
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1662588243)
This makes me wonder if `finalize=True` should result in `"complete": True`. It's another case of using `PSBTInputSigned()` instead of `PSBTInputSignedAndVerified()`. We could update this code below to automatically replace / update invalid signatures.
https://github.com/bitcoin/bitcoin/blob/d2c8d161b46bd62256a17abd086d8ae138c043c3/src/wallet/wallet.cpp#L2181-L2194
💬 pinheadmz commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1662597603)
There might be a simpler way to malleate the PSBT. You could at least skip the `getnewaddress` call and just hard-code something, or change one of the output amounts. Maybe only if you retouch.
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1662597603)
There might be a simpler way to malleate the PSBT. You could at least skip the `getnewaddress` call and just hard-code something, or change one of the output amounts. Maybe only if you retouch.
⚠️ iamvitalyo opened an issue: "Its O will solve your project."
(https://github.com/bitcoin/bitcoin/issues/30378)
### Please describe the feature you'd like to see added.
so have you normal AI to investigate my critic of no O. now I see x views max 17 so only team of x see what I am write all is what I want told is all in draft https://github.com/iamvitalyo/Aphrodite/
and
https://x.com/iamvitalyo
### Is your feature related to a problem, if so please describe it.
so have you normal AI to investigate my critic of no O. now I see x views max 17 so only team of x see what I am write all is what I want to
...
(https://github.com/bitcoin/bitcoin/issues/30378)
### Please describe the feature you'd like to see added.
so have you normal AI to investigate my critic of no O. now I see x views max 17 so only team of x see what I am write all is what I want told is all in draft https://github.com/iamvitalyo/Aphrodite/
and
https://x.com/iamvitalyo
### Is your feature related to a problem, if so please describe it.
so have you normal AI to investigate my critic of no O. now I see x views max 17 so only team of x see what I am write all is what I want to
...
✅ pinheadmz closed an issue: "Its O will solve your project."
(https://github.com/bitcoin/bitcoin/issues/30378)
(https://github.com/bitcoin/bitcoin/issues/30378)
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1662671837)
> would save a good number of unneeded checks in slow machines.
@furszy Let's say on a slow machine IBD takes 48 hours to sync, and being generous this check takes 10ms (I think in reality it would be more than 2 orders of magnitude faster), then the total number of checks is 48h * 60 minutes * 2 (twice a minute) = 5,760 checks * 10ms = 57.6 seconds. So on a 48 hour sync with a it will still be less than a minute extra time added.
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1662671837)
> would save a good number of unneeded checks in slow machines.
@furszy Let's say on a slow machine IBD takes 48 hours to sync, and being generous this check takes 10ms (I think in reality it would be more than 2 orders of magnitude faster), then the total number of checks is 48h * 60 minutes * 2 (twice a minute) = 5,760 checks * 10ms = 57.6 seconds. So on a 48 hour sync with a it will still be less than a minute extra time added.
💬 marcofleon commented on pull request "fuzz: fix ciphertext size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203409886)
I'm still getting an error with this patch.
```
FUZZ=crypter src/test/fuzz/fuzz crash-fee5221434486fefe1ec12410e7546625f82823a
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 4226801841
INFO: Loaded 1 modules (378324 inline 8-bit counters): 378324 [0x556f6632fb80, 0x556f6638c154),
INFO: Loaded 1 PC tables (378324 PCs): 378324 [0x556f6638c158,0x556f66951e98),
src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
Running: crash-fee5221434486fefe1ec12410e7546625
...
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203409886)
I'm still getting an error with this patch.
```
FUZZ=crypter src/test/fuzz/fuzz crash-fee5221434486fefe1ec12410e7546625f82823a
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 4226801841
INFO: Loaded 1 modules (378324 inline 8-bit counters): 378324 [0x556f6632fb80, 0x556f6638c154),
INFO: Loaded 1 PC tables (378324 PCs): 378324 [0x556f6638c158,0x556f66951e98),
src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
Running: crash-fee5221434486fefe1ec12410e7546625
...
💬 brunoerg commented on pull request "fuzz: fix ciphertext size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203464313)
Yes, just noticed more places need to be changed, working on it.
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203464313)
Yes, just noticed more places need to be changed, working on it.
💬 pinheadmz commented on issue "Unix Domain Sockets":
(https://github.com/bitcoin/bitcoin/issues/5029#issuecomment-2203469108)
Update: I attempted this but had an [issue with libevent](https://github.com/libevent/libevent/issues/1615). There is a patch on libevent master now but looks like they don't ship releases any more, and outside of depends builds, we can't guarantee the user has the patched version installed. I'm not even sure how we update our depends builds to pull libevent master branch instead of a release.
There seems to be some [core dev support ](https://www.erisian.com.au/bitcoin-core-dev/log-2024-04-1
...
(https://github.com/bitcoin/bitcoin/issues/5029#issuecomment-2203469108)
Update: I attempted this but had an [issue with libevent](https://github.com/libevent/libevent/issues/1615). There is a patch on libevent master now but looks like they don't ship releases any more, and outside of depends builds, we can't guarantee the user has the patched version installed. I'm not even sure how we update our depends builds to pull libevent master branch instead of a release.
There seems to be some [core dev support ](https://www.erisian.com.au/bitcoin-core-dev/log-2024-04-1
...
💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2203472829)
utACK 3bd618a91f
Code changes since last review look good.
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2203472829)
utACK 3bd618a91f
Code changes since last review look good.