💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1435850669)
Thanks updated
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1435850669)
Thanks updated
💬 fjahr commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868564680)
> If we are going to change it here, should also switch in `hash.cpp`
Done, thanks! I grepped for it didn't find this one, probably because of the all-caps.
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868564680)
> If we are going to change it here, should also switch in `hash.cpp`
Done, thanks! I grepped for it didn't find this one, probably because of the all-caps.
💬 GregTonoski commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1868893820)
Yes, the exact prefix in the file is:
```
##
## bitcoin.conf configuration file.
## Generated by contrib/devtools/gen-bitcoin-conf.sh.
##
## Lines beginning with # are comments.
## All possible configuration options are provided. To use, copy this file
## to your data directory (default or specified by -datadir), uncomment
## options you would like to change, and save the file.
##
```
I would suggest renaming the file, e.g. "example-bitcoin.conf".
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1868893820)
Yes, the exact prefix in the file is:
```
##
## bitcoin.conf configuration file.
## Generated by contrib/devtools/gen-bitcoin-conf.sh.
##
## Lines beginning with # are comments.
## All possible configuration options are provided. To use, copy this file
## to your data directory (default or specified by -datadir), uncomment
## options you would like to change, and save the file.
##
```
I would suggest renaming the file, e.g. "example-bitcoin.conf".
💬 dimitaracev commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868965467)
I can take a look at this.
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868965467)
I can take a look at this.
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1436080062)
>Alternatively, could use util::Result failure values after https://github.com/bitcoin/bitcoin/pull/25665 is merged.
will resolve this since I am now using `TxValidationState`
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1436080062)
>Alternatively, could use util::Result failure values after https://github.com/bitcoin/bitcoin/pull/25665 is merged.
will resolve this since I am now using `TxValidationState`
📝 22388o opened a pull request: "doc: upgrade Bitcoin Core license to 2024"
(https://github.com/bitcoin/bitcoin/pull/29142)
See [239450](https://github.com/bitcoin/bitcoin/pull/239450), [20805] (https://github.com/bitcoin/bitcoin/pull/20805), [17801](https://github.com/bitcoin/bitcoin/pull/17801), [15061](https://github.com/bitcoin/bitcoin/pull/15061) , [26748](https://github.com/bitcoin/bitcoin/pull/26748)
Every year had his upgrade :)
(https://github.com/bitcoin/bitcoin/pull/29142)
See [239450](https://github.com/bitcoin/bitcoin/pull/239450), [20805] (https://github.com/bitcoin/bitcoin/pull/20805), [17801](https://github.com/bitcoin/bitcoin/pull/17801), [15061](https://github.com/bitcoin/bitcoin/pull/15061) , [26748](https://github.com/bitcoin/bitcoin/pull/26748)
Every year had his upgrade :)
🤔 maflcko requested changes to a pull request: "doc: upgrade Bitcoin Core license to 2024"
(https://github.com/bitcoin/bitcoin/pull/29142#pullrequestreview-1795889686)
It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
(https://github.com/bitcoin/bitcoin/pull/29142#pullrequestreview-1795889686)
It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
💬 maflcko commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868987998)
The error seems to be: ` node0 stderr bitcoin-node: wallet/wallet.cpp:246: void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion `it.second' failed. `?
So I think another bug would be that the error is not caught and reported by the python code?
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868987998)
The error seems to be: ` node0 stderr bitcoin-node: wallet/wallet.cpp:246: void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion `it.second' failed. `?
So I think another bug would be that the error is not caught and reported by the python code?
💬 22388o commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989208)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
Ok
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989208)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
Ok
💬 22388o commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989343)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
So I can close PR?
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989343)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.
So I can close PR?
💬 maflcko commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868992264)
I guess this is fine, because the shift is constant and the compiler can skip the expensive modulo?
https://github.com/llvm/llvm-project/blob/9fba1d5f3a52af0ae62f386d0c494bd9510fa845/libcxx/include/__bit/rotate.h#L35
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868992264)
I guess this is fine, because the shift is constant and the compiler can skip the expensive modulo?
https://github.com/llvm/llvm-project/blob/9fba1d5f3a52af0ae62f386d0c494bd9510fa845/libcxx/include/__bit/rotate.h#L35
💬 maflcko commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868992681)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868992681)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 dimitaracev commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868993395)
Maybe we should return a proper message that indicates that the wallet we are trying to unload is already marked for unloading and we can catch that in the python code.
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868993395)
Maybe we should return a proper message that indicates that the wallet we are trying to unload is already marked for unloading and we can catch that in the python code.
💬 sipa commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1869001285)
@maflcko Constant propagation + strength reduction should make that code not emit an actual modulo operation in any compiler from the last 20 years.
But hopefully that pattern in libc++ also matches whatever pattern matching clang has to make sure it instead emits an actual rot instruction on platforms where it exists.
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1869001285)
@maflcko Constant propagation + strength reduction should make that code not emit an actual modulo operation in any compiler from the last 20 years.
But hopefully that pattern in libc++ also matches whatever pattern matching clang has to make sure it instead emits an actual rot instruction on platforms where it exists.
💬 Lajandra commented on issue "ci: failure in `wallet_basic.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29110#issuecomment-1869042800)
> https://cirrus-ci.com/task/5898005006516224?logs=ci#L2515 :
>
> ```shell
> _20231218_134952/wallet_basic_259/node0/regtest/wallets/default_wallet/wallet.dat] SQLite Statement: ROLLBACK TRANSACTION
> node0 2023-12-18T13:53:29.193915Z [scheduler] [wallet/sqlite.cpp:400] [Close] SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted
> node0 2023-12-18T13:53:29.194038Z [scheduler] [wallet/sqlite.cpp:53] [TraceSqlCallback] [/ci_container_base/
...
(https://github.com/bitcoin/bitcoin/issues/29110#issuecomment-1869042800)
> https://cirrus-ci.com/task/5898005006516224?logs=ci#L2515 :
>
> ```shell
> _20231218_134952/wallet_basic_259/node0/regtest/wallets/default_wallet/wallet.dat] SQLite Statement: ROLLBACK TRANSACTION
> node0 2023-12-18T13:53:29.193915Z [scheduler] [wallet/sqlite.cpp:400] [Close] SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted
> node0 2023-12-18T13:53:29.194038Z [scheduler] [wallet/sqlite.cpp:53] [TraceSqlCallback] [/ci_container_base/
...
💬 pinheadmz commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1869080735)
Ok I don't agree that that would be an improvement, because:
1. The path already contains the word "example" (`examples/bitcoin.conf`)
2. Changing that file's name would also require an additional comment in the header instructing the user to rename the file as well as copy it to their data directory
3. The default datadir for all platforms is very clear in several places in the docs, and that default datadir is never *inside* the code repository
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1869080735)
Ok I don't agree that that would be an improvement, because:
1. The path already contains the word "example" (`examples/bitcoin.conf`)
2. Changing that file's name would also require an additional comment in the header instructing the user to rename the file as well as copy it to their data directory
3. The default datadir for all platforms is very clear in several places in the docs, and that default datadir is never *inside* the code repository
💬 kashifs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#issuecomment-1869114446)
Concept ACK
I compiled from source, read through each modified line of code, ran the automated tests, and modified `test_maxfeerate_maxburn_submitpackage` test to check `maxfeerate` and `maxburnamount` seperately
(https://github.com/bitcoin/bitcoin/pull/28950#issuecomment-1869114446)
Concept ACK
I compiled from source, read through each modified line of code, ran the automated tests, and modified `test_maxfeerate_maxburn_submitpackage` test to check `maxfeerate` and `maxburnamount` seperately
💬 BitcoinMechanic commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1869194444)
Strong ACK.
1. It makes bloating the UTXO set trivial and this is being done with the recent "stamps" spam attack.
2. It's a poor way to do multisig (and arguments for stateless multisig aren't valid - it's not the responsibility of users running nodes to store data only relevant to you.)
It makes sense for nodes to refuse to relay unnecessarily burdensome TXs.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1869194444)
Strong ACK.
1. It makes bloating the UTXO set trivial and this is being done with the recent "stamps" spam attack.
2. It's a poor way to do multisig (and arguments for stateless multisig aren't valid - it's not the responsibility of users running nodes to store data only relevant to you.)
It makes sense for nodes to refuse to relay unnecessarily burdensome TXs.
💬 BitcoinMechanic commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1869197720)
Filtering is a very useful step. It is irrelevant to continually bring up the point that miners can still include this stuff in blocks.
Miners get to decide what goes in blocks and if the wider network is strongly indicating that it (correctly) considers a use-case to be undesirable they can choose to mine templates that do not include such "transactions". Until now there has been no signalling of this and no good option beyond solo mining. Ocean pool (disclaimer: my employer) provides this o
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1869197720)
Filtering is a very useful step. It is irrelevant to continually bring up the point that miners can still include this stuff in blocks.
Miners get to decide what goes in blocks and if the wider network is strongly indicating that it (correctly) considers a use-case to be undesirable they can choose to mine templates that do not include such "transactions". Until now there has been no signalling of this and no good option beyond solo mining. Ocean pool (disclaimer: my employer) provides this o
...
💬 da2ce7 commented on pull request "Ephemeral Anchors":
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-1869288197)
@luke-jr
> > BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki
>
> Node policy is not a subject matter for standardization in itself. It's the transaction format that the BIP (if there is one) should focus on, not speculative behaviour of individual nodes.
This Proposed BIP brings a set of concepts that allows the community to communicate using a coherent language regarding what "Ephemeral Anchors" are. By defining this term in conte
...
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-1869288197)
@luke-jr
> > BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki
>
> Node policy is not a subject matter for standardization in itself. It's the transaction format that the BIP (if there is one) should focus on, not speculative behaviour of individual nodes.
This Proposed BIP brings a set of concepts that allows the community to communicate using a coherent language regarding what "Ephemeral Anchors" are. By defining this term in conte
...