💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1214751567)
Orphan usage would effect the CPFP transactions, IIUC, not the commitment transactions. The CPFP is deemed high enough, sent to the peer, the peer holds the child tx in orphan set and requests the full package, which isn't size-bound(beyond current mempool limits).
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1214751567)
Orphan usage would effect the CPFP transactions, IIUC, not the commitment transactions. The CPFP is deemed high enough, sent to the peer, the peer holds the child tx in orphan set and requests the full package, which isn't size-bound(beyond current mempool limits).
💬 brunoerg commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1214786122)
nit:
```suggestion
if blockmintxfee_btc_kvb > 0: # can't go below zero-fee
```
I think `>` fits better with the comment "can't go below zero-fee" then `!=`
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1214786122)
nit:
```suggestion
if blockmintxfee_btc_kvb > 0: # can't go below zero-fee
```
I think `>` fits better with the comment "can't go below zero-fee" then `!=`
👍 brunoerg approved a pull request: "test: miner: add coverage for `-blockmintxfee` setting"
(https://github.com/bitcoin/bitcoin/pull/27620#pullrequestreview-1458443810)
crACK 989fa37670cbb386873a765799fd6db8ea6986f7
Nice test coverage!
Just a thought that came to my mind while reviewing it:
- Seems like these test coverage uses just 1 node, if we'd stop node1 right before line 81, it wouldn't make the test to fail. So, it seems that `-minrelaytxfee=0` is just to make `send_self_transfer` - which calls `sendrawtransaction` - not fail (even if we might not have any connection).
(https://github.com/bitcoin/bitcoin/pull/27620#pullrequestreview-1458443810)
crACK 989fa37670cbb386873a765799fd6db8ea6986f7
Nice test coverage!
Just a thought that came to my mind while reviewing it:
- Seems like these test coverage uses just 1 node, if we'd stop node1 right before line 81, it wouldn't make the test to fail. So, it seems that `-minrelaytxfee=0` is just to make `send_self_transfer` - which calls `sendrawtransaction` - not fail (even if we might not have any connection).
💬 brunoerg commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1574302283)
Correct me please if I'm missing anything. Considering:
```
- Once we get an INV from somebody, request the transaction with GETDATA, as if we didn't have it before.
- After we receive the full transaction as a TX message, in reply to our GETDATA request, only then consider the transaction has propagated through the network and remove it from the unbroadcast list.
```
and
```
We could wait for more than one INV, from peers that we have selected (outbound) and that are specifically not t
...
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1574302283)
Correct me please if I'm missing anything. Considering:
```
- Once we get an INV from somebody, request the transaction with GETDATA, as if we didn't have it before.
- After we receive the full transaction as a TX message, in reply to our GETDATA request, only then consider the transaction has propagated through the network and remove it from the unbroadcast list.
```
and
```
We could wait for more than one INV, from peers that we have selected (outbound) and that are specifically not t
...
💬 brunoerg commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214831451)
It seems for extra anonymity, but I have a similar doubt, couldn't I make easier for someone to censor us?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214831451)
It seems for extra anonymity, but I have a similar doubt, couldn't I make easier for someone to censor us?
💬 instagibbs commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214832859)
unless we want to sybil the network and fake long-term connections, it's obvious what this connection will be about? Just retry with new peers until someone honest propagates it?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214832859)
unless we want to sybil the network and fake long-term connections, it's obvious what this connection will be about? Just retry with new peers until someone honest propagates it?
💬 ryanofsky commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574316464)
> Turning this on will log private keys as they are being written to the db, which seems a little scary.
Yes when the tracing was turned on, having access to the debug log file would be like having access to the database file. The keys would at least be encrypted if the wallet had a password set, but it could still be scary/unexpected behavior.
I looked into ways of trying to mask values in the sql statement, but the `sqlite3_stmt` object is pretty opaque and I couldn't find any function h
...
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574316464)
> Turning this on will log private keys as they are being written to the db, which seems a little scary.
Yes when the tracing was turned on, having access to the debug log file would be like having access to the database file. The keys would at least be encrypted if the wallet had a password set, but it could still be scary/unexpected behavior.
I looked into ways of trying to mask values in the sql statement, but the `sqlite3_stmt` object is pretty opaque and I couldn't find any function h
...
💬 achow101 commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574364005)
ACK ff9d961bf38b24f8f931dcf66799cbc468e473df
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574364005)
ACK ff9d961bf38b24f8f931dcf66799cbc468e473df
💬 achow101 commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1214886372)
In bbb927739955b1588d94c5fd8ed73ddf5afde46c "fuzz: Fix mini_miner_selection running out of coin"
Is it possible that `available_coins` is empty and so `num_inputs = 0`? It may be useful to have an additional assert before the inputs are filled below.
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1214886372)
In bbb927739955b1588d94c5fd8ed73ddf5afde46c "fuzz: Fix mini_miner_selection running out of coin"
Is it possible that `available_coins` is empty and so `num_inputs = 0`? It may be useful to have an additional assert before the inputs are filled below.
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214935160)
Thank you, I added the delete attempt
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214935160)
Thank you, I added the delete attempt
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214936512)
Sure, thank you, I updated the test.
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214936512)
Sure, thank you, I updated the test.
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1574425436)
> Also wondering if we should include a hidden argument to allow older files for testing reasons, re: #27415
> I've used it for systems I've built for integration testing, and not having to touch a file during testing to refresh it would be a simplification.
>
> > For testing, they could mock the time right?
>
> Depends on your test setup, really. It might interfere during integration tests which require a realistic clock?
>
> Makes sense, seems fine as a regtest-only option for testing
...
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1574425436)
> Also wondering if we should include a hidden argument to allow older files for testing reasons, re: #27415
> I've used it for systems I've built for integration testing, and not having to touch a file during testing to refresh it would be a simplification.
>
> > For testing, they could mock the time right?
>
> Depends on your test setup, really. It might interfere during integration tests which require a realistic clock?
>
> Makes sense, seems fine as a regtest-only option for testing
...
💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1215190508)
I think the answer is just to add a new tx entry field for the steady clock time and use that at natural precision.
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1215190508)
I think the answer is just to add a new tx entry field for the steady clock time and use that at natural precision.
💬 desibitcoiner commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1574690664)
I'm trying to run a build on Fedora 36 and I'm seeing the same error.
When I run ./configure I see the following:
configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
configure: WARNING: Passing --without-bdb will suppress this warning
checking for SQLITE... yes
checking whether to build wallet with support for sqlite... yes
checking whether Userspace, Statically Defined Tracing tracepoints are supported... no
checking for natpmp.h... yes
checking for
...
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1574690664)
I'm trying to run a build on Fedora 36 and I'm seeing the same error.
When I run ./configure I see the following:
configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
configure: WARNING: Passing --without-bdb will suppress this warning
checking for SQLITE... yes
checking whether to build wallet with support for sqlite... yes
checking whether Userspace, Statically Defined Tracing tracepoints are supported... no
checking for natpmp.h... yes
checking for
...
⚠️ desibitcoiner opened an issue: "Build fails on Fedora 36 - configure failed for src/secp256k1"
(https://github.com/bitcoin/bitcoin/issues/27808)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Found an recent similar issue but didn't help:
https://github.com/bitcoin/bitcoin/issues/27550
I'm trying to run a build on Fedora 36 and I'm seeing the same error.
When I run ./configure I see the following:
configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
configure: WARNING: Passing --without-bdb will suppress this warning
checking for SQLI
...
(https://github.com/bitcoin/bitcoin/issues/27808)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Found an recent similar issue but didn't help:
https://github.com/bitcoin/bitcoin/issues/27550
I'm trying to run a build on Fedora 36 and I'm seeing the same error.
When I run ./configure I see the following:
configure: WARNING: Bitcoin Core requires this library for BDB (legacy) wallet support
configure: WARNING: Passing --without-bdb will suppress this warning
checking for SQLI
...
👍 hebasto approved a pull request: "guix: remove cURL from build env"
(https://github.com/bitcoin/bitcoin/pull/27779#pullrequestreview-1458997060)
ACK 641897a83dc9d40b618efbae67c3beb90a1f34f8
Guix build:
```
7e9b3758aadd87db9c89fec5c05ca7a5020aaebf185daee4f7570dd1eeb23355 guix-build-641897a83dc9/output/aarch64-linux-gnu/SHA256SUMS.part
494f06d1f21ad20aa8cc8f61c4dfeda053215f41bb2aa0bfd47343909edf4dad guix-build-641897a83dc9/output/aarch64-linux-gnu/bitcoin-641897a83dc9-aarch64-linux-gnu-debug.tar.gz
8a3912a0ff0cb41b87494291ca13bc9caa47ff08d57f37b6d99a9c8e3667390a guix-build-641897a83dc9/output/aarch64-linux-gnu/bitcoin-641897a83dc
...
(https://github.com/bitcoin/bitcoin/pull/27779#pullrequestreview-1458997060)
ACK 641897a83dc9d40b618efbae67c3beb90a1f34f8
Guix build:
```
7e9b3758aadd87db9c89fec5c05ca7a5020aaebf185daee4f7570dd1eeb23355 guix-build-641897a83dc9/output/aarch64-linux-gnu/SHA256SUMS.part
494f06d1f21ad20aa8cc8f61c4dfeda053215f41bb2aa0bfd47343909edf4dad guix-build-641897a83dc9/output/aarch64-linux-gnu/bitcoin-641897a83dc9-aarch64-linux-gnu-debug.tar.gz
8a3912a0ff0cb41b87494291ca13bc9caa47ff08d57f37b6d99a9c8e3667390a guix-build-641897a83dc9/output/aarch64-linux-gnu/bitcoin-641897a83dc
...
📝 aguycalled opened a pull request: "BLSCT SubAddressPool"
(https://github.com/bitcoin/bitcoin/pull/27809)
This PR adds support for the use of a SubAddress pool and the generation of different receiving addresses.
`getnewaddress` RPC method pulls addresses from the pool and shows to the requester receiving addresses.
By default it generates addresses under account `0`.
(https://github.com/bitcoin/bitcoin/pull/27809)
This PR adds support for the use of a SubAddress pool and the generation of different receiving addresses.
`getnewaddress` RPC method pulls addresses from the pool and shows to the requester receiving addresses.
By default it generates addresses under account `0`.
✅ fanquake closed a pull request: "BLSCT SubAddressPool"
(https://github.com/bitcoin/bitcoin/pull/27809)
(https://github.com/bitcoin/bitcoin/pull/27809)
💬 Ayms commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1574929624)
I must be brain damaged or something like this, still I don't get what it has to do with https://github.com/bitcoin/bitcoin/issues/27043 (and when it will happen)
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1574929624)
I must be brain damaged or something like this, still I don't get what it has to do with https://github.com/bitcoin/bitcoin/issues/27043 (and when it will happen)
📝 dergoegge opened a pull request: "fuzz: Partially revert #27780"
(https://github.com/bitcoin/bitcoin/pull/27810)
Partially revert #27780, because moving the duplicate coinbase check out of the `was_valid` branch leads to non-bug crashes in the fuzz target.
For context and further explanation see: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59516
(https://github.com/bitcoin/bitcoin/pull/27810)
Partially revert #27780, because moving the duplicate coinbase check out of the `was_valid` branch leads to non-bug crashes in the fuzz target.
For context and further explanation see: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59516