💬 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.
💬 instagibbs commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2203545930)
reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
reviewed via "git range-diff master a6821a0 926b8e3"
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2203545930)
reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
reviewed via "git range-diff master a6821a0 926b8e3"
💬 spacebear21 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2203628213)
Thanks @willcl-ark for picking this up! Confirmed this fixes the CHECK_NONFATAL abort when a witness signature is invalid.
tACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2203628213)
Thanks @willcl-ark for picking this up! Confirmed this fixes the CHECK_NONFATAL abort when a witness signature is invalid.
tACK f57f68ba95fc8778fe2dc755da1e631fe60592c4
💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203675621)
> I think the last commit [064a401](https://github.com/bitcoin/bitcoin/commit/064a401e10f2b2da8f31f682929431d06c671d93) is unnecessary scope creep for this PR, and although it looks safe to me, I am not 100% confident I am grasping the full extent of the implications, and I think a separate PR would've been more suitable.
Yeah, on second thought, I agree. My bad. I'd also feel more comfortable ACKing without it.
@TheCharlatan want to pop that one off?
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203675621)
> I think the last commit [064a401](https://github.com/bitcoin/bitcoin/commit/064a401e10f2b2da8f31f682929431d06c671d93) is unnecessary scope creep for this PR, and although it looks safe to me, I am not 100% confident I am grasping the full extent of the implications, and I think a separate PR would've been more suitable.
Yeah, on second thought, I agree. My bad. I'd also feel more comfortable ACKing without it.
@TheCharlatan want to pop that one off?
💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203689268)
Force-pushed changing more sizes, I updated the PR description with the changes.
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203689268)
Force-pushed changing more sizes, I updated the PR description with the changes.
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203785402)
Updated 45e0c96e81b5f0c364af717b7cb2d9bc094372de -> 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab ([noGlobalScriptCache_11](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_11) -> [noGlobalScriptCache_12](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_11..noGlobalScriptCache_12))
* Dropped the last commit as requested by @stickies-v and @theuni
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203785402)
Updated 45e0c96e81b5f0c364af717b7cb2d9bc094372de -> 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab ([noGlobalScriptCache_11](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_11) -> [noGlobalScriptCache_12](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_11..noGlobalScriptCache_12))
* Dropped the last commit as requested by @stickies-v and @theuni