π¬ Symphonic3 commented on issue "Change estimate_mode default to "ECONOMICAL" in these RPC calls":
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2089182279)
Willing to create a PR for this if desired!
(https://github.com/bitcoin/bitcoin/issues/30009#issuecomment-2089182279)
Willing to create a PR for this if desired!
π cbergqvist approved a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-2034535106)
reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good!
Tested on RAM disk of 1GB, 1430MB (`doas mount -t tmpfs -o size=1430m tmpfs /mnt/tmp/`) and varied the `--jobs` and `--nocleanup`, observing expected warnings + error on disk full.
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-2034535106)
reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good!
Tested on RAM disk of 1GB, 1430MB (`doas mount -t tmpfs -o size=1430m tmpfs /mnt/tmp/`) and varied the `--jobs` and `--nocleanup`, observing expected warnings + error on disk full.
π¬ AngusP commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586884665)
(Ignore me if I'm understanding, still picking things up π ) -- coinbase transactions are [required to have all `0` Wtxids](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#commitment-structure) (`0x0000....0000`) which means for an `INV` message for a witness coinbase you have to use the non-witness txid to reference the transaction? - so `AlreadyHaveTx` in that case is explicitly *not* using a Wtxid? (Or are coinbases never `INV`'d?)
Also perhaps not great/confusing to treat W
...
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586884665)
(Ignore me if I'm understanding, still picking things up π ) -- coinbase transactions are [required to have all `0` Wtxids](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#commitment-structure) (`0x0000....0000`) which means for an `INV` message for a witness coinbase you have to use the non-witness txid to reference the transaction? - so `AlreadyHaveTx` in that case is explicitly *not* using a Wtxid? (Or are coinbases never `INV`'d?)
Also perhaps not great/confusing to treat W
...
π¬ andrewtoth commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2089285750)
@sr-gi thank you for your review!
> that we should followup this with sending NotFounds for all the blocks that we are unable (or unwilling) to provide
After https://github.com/bitcoin/bitcoin/pull/28120 I don't think it is very likely for this to occur by updated nodes. For older nodes and other software that requests blocks past the prune threshold it would require a protocol change as @sipa mentioned.
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2089285750)
@sr-gi thank you for your review!
> that we should followup this with sending NotFounds for all the blocks that we are unable (or unwilling) to provide
After https://github.com/bitcoin/bitcoin/pull/28120 I don't think it is very likely for this to occur by updated nodes. For older nodes and other software that requests blocks past the prune threshold it would require a protocol change as @sipa mentioned.
π hebasto opened a pull request: "refactor, fuzz: Make 64-bit shift explicit"
(https://github.com/bitcoin/bitcoin/pull/30017)
This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to https://github.com/bitcoin/bitcoin/pull/26252.
All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
Required to simplify warning suppression porting to the CMake-based build system.
(https://github.com/bitcoin/bitcoin/pull/30017)
This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to https://github.com/bitcoin/bitcoin/pull/26252.
All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
Required to simplify warning suppression porting to the CMake-based build system.
π¬ furszy commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2089357427)
Little add about the notfound response; I already have a draft for it. I'm busy with other stuff this week but will open the PR next week :).
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-2089357427)
Little add about the notfound response; I already have a draft for it. I'm busy with other stuff this week but will open the PR next week :).
π ajtowns opened a pull request: "Implement BIP 118 validation (SIGHASH_ANYPREVOUT)"
(https://github.com/bitcoin/bitcoin/pull/30018)
Forward port of #34
(https://github.com/bitcoin/bitcoin/pull/30018)
Forward port of #34
β
ajtowns closed a pull request: "Implement BIP 118 validation (SIGHASH_ANYPREVOUT)"
(https://github.com/bitcoin/bitcoin/pull/30018)
(https://github.com/bitcoin/bitcoin/pull/30018)
π¬ ajtowns commented on pull request "Implement BIP 118 validation (SIGHASH_ANYPREVOUT)":
(https://github.com/bitcoin/bitcoin/pull/30018#issuecomment-2089358941)
Grr :(
(https://github.com/bitcoin/bitcoin/pull/30018#issuecomment-2089358941)
Grr :(
π¬ fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2089372464)
It's waiting for the PR linked in the description. Otherwise, should be mergable.
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2089372464)
It's waiting for the PR linked in the description. Otherwise, should be mergable.
π¬ ariard commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2089386661)
> Would this make things {broken, inconvenient} for potential users of TRUCs, e.g. by requiring swapping between v3 and v2?
For historical lightning channel who have already negotiated parameters above `max_accepted_htlcs` + witness spend size of legacy channel types. i.e anything before `option_anchors_zero_fee_htlc_tx`, I think. Legacy v2 commitment transactions are already opting to RBF, so shall be able to be replaced by v3. This is unclear for the splicing flow, where a counterparty coul
...
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2089386661)
> Would this make things {broken, inconvenient} for potential users of TRUCs, e.g. by requiring swapping between v3 and v2?
For historical lightning channel who have already negotiated parameters above `max_accepted_htlcs` + witness spend size of legacy channel types. i.e anything before `option_anchors_zero_fee_htlc_tx`, I think. Legacy v2 commitment transactions are already opting to RBF, so shall be able to be replaced by v3. This is unclear for the splicing flow, where a counterparty coul
...
π¬ kevkevinpal commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586967904)
instead of number `298` would it make more sense to use `SNAPSHOT_BASE_HEIGHT`?
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586967904)
instead of number `298` would it make more sense to use `SNAPSHOT_BASE_HEIGHT`?
π¬ kevkevinpal commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586968840)
is this not the same as `START_HEIGHT`?
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586968840)
is this not the same as `START_HEIGHT`?
π¬ kevkevinpal commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586972804)
```suggestion
block_hash = node.getblockhash(START_HEIGHT + 1)
node.invalidateblock(block_hash)
assert_equal(node.getblockcount(), START_HEIGHT)
```
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1586972804)
```suggestion
block_hash = node.getblockhash(START_HEIGHT + 1)
node.invalidateblock(block_hash)
assert_equal(node.getblockcount(), START_HEIGHT)
```
π kevkevinpal opened a pull request: "test: create assert_less_than util"
(https://github.com/bitcoin/bitcoin/pull/30019)
In the functional tests there are lots of cases where we assert < which we now swap with assert_less_than to be more readable
This is motivated/uses logic from this PR which was closed https://github.com/bitcoin/bitcoin/pull/28528
This partially helps https://github.com/bitcoin/bitcoin/issues/23119
I've broken it up to just assert_less_than to keep the PR smaller as suggested in https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805
[Similar change for assert_not_equal](
...
(https://github.com/bitcoin/bitcoin/pull/30019)
In the functional tests there are lots of cases where we assert < which we now swap with assert_less_than to be more readable
This is motivated/uses logic from this PR which was closed https://github.com/bitcoin/bitcoin/pull/28528
This partially helps https://github.com/bitcoin/bitcoin/issues/23119
I've broken it up to just assert_less_than to keep the PR smaller as suggested in https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805
[Similar change for assert_not_equal](
...
π¬ ariard commented on pull request "chainparams: Add achow101 DNS seeder":
(https://github.com/bitcoin/bitcoin/pull/30007#issuecomment-2089436259)
Concept ACK.
By the way, it would be great if mainnet DNS seeders are considering to sign by default the peers records. This could be amply checked in `ThreadDNSAddressSeed()` or in semi-automatic fashion in the logs. ECDSA secp256k1 isnβt supported by default in DNS, though you have ed25519 or ECDSA P256 which are widely supported by key management softwares.
(https://github.com/bitcoin/bitcoin/pull/30007#issuecomment-2089436259)
Concept ACK.
By the way, it would be great if mainnet DNS seeders are considering to sign by default the peers records. This could be amply checked in `ThreadDNSAddressSeed()` or in semi-automatic fashion in the logs. ECDSA secp256k1 isnβt supported by default in DNS, though you have ed25519 or ECDSA P256 which are widely supported by key management softwares.
π fanquake merged a pull request: "depends: build miniupnpc with CMake"
(https://github.com/bitcoin/bitcoin/pull/29707)
(https://github.com/bitcoin/bitcoin/pull/29707)
π¬ fanquake commented on pull request "[WIP] libevent @ master + use CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2089471495)
Rebased. Dropped the final patch commit, as that change happened upstream. Also bumped to current master.
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2089471495)
Rebased. Dropped the final patch commit, as that change happened upstream. Also bumped to current master.
π¬ saadbitcoin commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2089476288)
Can you healp me
ΩΩ Ψ§ΩΨ£ΨΨ―Ψ Ω’Ω¨ Ψ£Ψ¨Ψ±ΩΩ Ω’Ω Ω’Ω€ Ω§:Ω’Ω¦ Ω 1440000bytes ***@***.***> ΩΨͺΨ¨:
> Tested it and I think it can be improved further before users can try it
> on mainnet: bitcoin-core/gui#733 (comment)
> <https://github.com/bitcoin-core/gui/pull/733#issuecomment-2081539227>
>
> β
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2081540237>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHV67JU42HUR
...
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2089476288)
Can you healp me
ΩΩ Ψ§ΩΨ£ΨΨ―Ψ Ω’Ω¨ Ψ£Ψ¨Ψ±ΩΩ Ω’Ω Ω’Ω€ Ω§:Ω’Ω¦ Ω 1440000bytes ***@***.***> ΩΨͺΨ¨:
> Tested it and I think it can be improved further before users can try it
> on mainnet: bitcoin-core/gui#733 (comment)
> <https://github.com/bitcoin-core/gui/pull/733#issuecomment-2081539227>
>
> β
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2081540237>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHV67JU42HUR
...
π fanquake merged a pull request: "lint: [doc] Clarify Windows line endings (CR LF) not to be used"
(https://github.com/bitcoin/bitcoin/pull/30010)
(https://github.com/bitcoin/bitcoin/pull/30010)