π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920206)
Thanks, fixed
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920206)
Thanks, fixed
π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920679)
Thanks, fixed
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920679)
Thanks, fixed
π 1alexbc1 approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2145858736)
Amadeevantv yimelaja
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2145858736)
Amadeevantv yimelaja
π¬ ariard commented on issue "Brainstorm: Transaction issuer-selected policy limits":
(https://github.com/bitcoin/bitcoin/issues/29454#issuecomment-2195109286)
> The problem with policy limits is they're all based on asking miners to ignore transactions that are potentially profitable. This is similar in spirit to opt-in RBF. And as we've seen, the supermajority of hash power eventually decided to ignore the opt-in flag and just do RBF all the time. If push came to shove, it wouldn't be surprising to run into the same class of problem with policy limits.
> Much better to focus on solutions like replace-by-fee-rate that rely on better aligning miner
...
(https://github.com/bitcoin/bitcoin/issues/29454#issuecomment-2195109286)
> The problem with policy limits is they're all based on asking miners to ignore transactions that are potentially profitable. This is similar in spirit to opt-in RBF. And as we've seen, the supermajority of hash power eventually decided to ignore the opt-in flag and just do RBF all the time. If push came to shove, it wouldn't be surprising to run into the same class of problem with policy limits.
> Much better to focus on solutions like replace-by-fee-rate that rely on better aligning miner
...
π¬ furszy commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657420121)
> Iβm not familiar with IOWY and could not turn up anything with a websearch, what is that?
Upps, the acronym is IWYU from include-what-you-use. I wrote it fast as IOWY "include only what you use".
Should stop using acronyms..
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657420121)
> Iβm not familiar with IOWY and could not turn up anything with a websearch, what is that?
Upps, the acronym is IWYU from include-what-you-use. I wrote it fast as IOWY "include only what you use".
Should stop using acronyms..
π¬ Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2195114617)
You can also drop the `BerkeleyDatabaseSanityCheck` related suppression in `contrib/devtools/check-devs.sh`
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2195114617)
You can also drop the `BerkeleyDatabaseSanityCheck` related suppression in `contrib/devtools/check-devs.sh`
π¬ darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2195118674)
Alright, i can't reproduce the non-determinism he observed.
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2195118674)
Alright, i can't reproduce the non-determinism he observed.
π€ furszy reviewed a pull request: "Wallet: Add `max_tx_weight` to transaction funding options (take 2)"
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2145904445)
utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2145904445)
utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6
π¬ furszy commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1657436891)
In 734076c6de178:
nano nit: shorter
```suggestion
if (*coin_selection_params.m_max_tx_weight < minimum_tx_weight || *coin_selection_params.m_max_tx_weight > MAX_STANDARD_TX_WEIGHT) {
```
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1657436891)
In 734076c6de178:
nano nit: shorter
```suggestion
if (*coin_selection_params.m_max_tx_weight < minimum_tx_weight || *coin_selection_params.m_max_tx_weight > MAX_STANDARD_TX_WEIGHT) {
```
π¬ mjdietzx commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2195192807)
Tested ACK cb41e26eef3a92a7704a0e01eb9b272f7ed78eeb. Note: I did not do a thorough code review
What I've tested:
- `getdescriptorinfo`, `importdescriptors`, `listdescriptors` with a miniscript wallet similar to [#29156](https://github.com/bitcoin/bitcoin/pull/29156). All works as expected
- import this wallet from file to Coldcard Mk4 (Edge FW 6.2.2 w/ miniscript support)
- `<0;1>` notation works properly as Coldcard has already adopted this convention
- Coldcard also signs psbt prope
...
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2195192807)
Tested ACK cb41e26eef3a92a7704a0e01eb9b272f7ed78eeb. Note: I did not do a thorough code review
What I've tested:
- `getdescriptorinfo`, `importdescriptors`, `listdescriptors` with a miniscript wallet similar to [#29156](https://github.com/bitcoin/bitcoin/pull/29156). All works as expected
- import this wallet from file to Coldcard Mk4 (Edge FW 6.2.2 w/ miniscript support)
- `<0;1>` notation works properly as Coldcard has already adopted this convention
- Coldcard also signs psbt prope
...
π ternera opened a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351)
I made some minor changes to the project by cleaning up grammar and punctuation mistakes in the README.
This will make it easier for people to read it and increase the professionalism of repo since the README is the first thing people see.
(https://github.com/bitcoin/bitcoin/pull/30351)
I made some minor changes to the project by cleaning up grammar and punctuation mistakes in the README.
This will make it easier for people to read it and increase the professionalism of repo since the README is the first thing people see.
π€ maflcko reviewed a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351#pullrequestreview-2145972278)
Closing for now. Not sure those changes are an improvement. Thus, it may be too controversial.
(https://github.com/bitcoin/bitcoin/pull/30351#pullrequestreview-2145972278)
Closing for now. Not sure those changes are an improvement. Thus, it may be too controversial.
π¬ maflcko commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657477704)
not sure this is an improvement.
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657477704)
not sure this is an improvement.
β
maflcko closed a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351)
(https://github.com/bitcoin/bitcoin/pull/30351)
π¬ maflcko commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#issuecomment-2195220001)
If you are looking for other stuff to contribute to, see:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests.
...
(https://github.com/bitcoin/bitcoin/pull/30351#issuecomment-2195220001)
If you are looking for other stuff to contribute to, see:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests.
...
π¬ ternera commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657481230)
Definitely a typo. Shall I fix it and open a new PR?
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657481230)
Definitely a typo. Shall I fix it and open a new PR?
π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657406844)
Should be snake_case.
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657406844)
Should be snake_case.
π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411032)
Remove spaces after comments describing params.
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411032)
Remove spaces after comments describing params.
π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657408296)
`nInput` is unused, letβs remove it.
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657408296)
`nInput` is unused, letβs remove it.
π¬ murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411595)
Outdated comment, itβs an OutputGroup not coin now.
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411595)
Outdated comment, itβs an OutputGroup not coin now.