💬 hebasto commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2186368722)
@techy2
> taking a look at focal32 expected path for cc1, it is not there. Installed gcc 10.5.0 once again and succeeded in populating /usr/lib/gcc/i686-linux-gnu/10/cc1 works now on focal32
>
> However the failure to cross compile on the x64 platform is an issue
>
> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu from the config log does not look right to me for an 1686 cross build
Do I understand correctly that you has resolved the initial issue mentione
...
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2186368722)
@techy2
> taking a look at focal32 expected path for cc1, it is not there. Installed gcc 10.5.0 once again and succeeded in populating /usr/lib/gcc/i686-linux-gnu/10/cc1 works now on focal32
>
> However the failure to cross compile on the x64 platform is an issue
>
> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu from the config log does not look right to me for an 1686 cross build
Do I understand correctly that you has resolved the initial issue mentione
...
💬 stickies-v commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30305#discussion_r1650897241)
Sure, but that seems to be an undocumented exception? Since 0.10.0, it [seems](https://bitcoincore.org/bin/) we only have 3 (in the root dir) releases where we we had a non-zero patch number. We also don't mention patch versions in our [documentation](https://bitcoincore.org/en/lifecycle/).
Keeping the git tag to `.x` makes sense because it gives us the flexibility to release as a minor or a patch version, i.e. we can still easily update the release notes to `27.1.1` if necessary. It just see
...
(https://github.com/bitcoin/bitcoin/pull/30305#discussion_r1650897241)
Sure, but that seems to be an undocumented exception? Since 0.10.0, it [seems](https://bitcoincore.org/bin/) we only have 3 (in the root dir) releases where we we had a non-zero patch number. We also don't mention patch versions in our [documentation](https://bitcoincore.org/en/lifecycle/).
Keeping the git tag to `.x` makes sense because it gives us the flexibility to release as a minor or a patch version, i.e. we can still easily update the release notes to `27.1.1` if necessary. It just see
...
💬 fanquake commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30305#discussion_r1650958819)
Ok. Lets follow up with ways we might want to streamline this for `28.x`.
(https://github.com/bitcoin/bitcoin/pull/30305#discussion_r1650958819)
Ok. Lets follow up with ways we might want to streamline this for `28.x`.
👍 rkrux approved a pull request: "Wallet: Add `max_tx_weight` to transaction funding options (take 2)"
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2135369361)
Concept ACK [723a8da](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe), but I would look at [this](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe) commit a bit more again; `make, test/functional` are successful though.
Following transaction funding RPCs are affected with the addition of `max_tx_weight` option:
1. `fundrawtransaction`
2. `walletcreatefundedpsbt`
3. `send`
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2135369361)
Concept ACK [723a8da](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe), but I would look at [this](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe) commit a bit more again; `make, test/functional` are successful though.
Following transaction funding RPCs are affected with the addition of `max_tx_weight` option:
1. `fundrawtransaction`
2. `walletcreatefundedpsbt`
3. `send`
💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650773911)
As per the following 2 statements in `ChooseSelectionResult`:
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L699
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L709
The `no_input` and `change_output` weights are accounted for, but where is the weight of the destination output accounted for?
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650773911)
As per the following 2 statements in `ChooseSelectionResult`:
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L699
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L709
The `no_input` and `change_output` weights are accounted for, but where is the weight of the destination output accounted for?
💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650938860)
Was the intention to write "In order to ..."?
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650938860)
Was the intention to write "In order to ..."?
💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650867991)
> This change ensures consistency in transaction size and weight calculation
within the wallet and prevents conversion overflow when calculating
`max_selection_weight`.
Are the inconsistencies because size_t is platform dependent and int is not (at least for the purposes of core because of this: https://github.com/bitcoin/bitcoin/blob/v26.0/src/compat/assumptions.h#L44)?
I still need to digest this comment: https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1528745048
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650867991)
> This change ensures consistency in transaction size and weight calculation
within the wallet and prevents conversion overflow when calculating
`max_selection_weight`.
Are the inconsistencies because size_t is platform dependent and int is not (at least for the purposes of core because of this: https://github.com/bitcoin/bitcoin/blob/v26.0/src/compat/assumptions.h#L44)?
I still need to digest this comment: https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1528745048
💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650862134)
Took me a while to figure but as I see they are accounted for within `coin_selection_params.tx_noinputs_size` here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L1100
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650862134)
Took me a while to figure but as I see they are accounted for within `coin_selection_params.tx_noinputs_size` here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L1100
💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650936364)
Nit: In case the file is changed.
```
# ensure the transaction's weight is below the specified max_tx_weight.
```
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650936364)
Nit: In case the file is changed.
```
# ensure the transaction's weight is below the specified max_tx_weight.
```
🚀 fanquake merged a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30305)
(https://github.com/bitcoin/bitcoin/pull/30305)
🚀 fanquake merged a pull request: "[26.x] upnp: fix build with miniupnpc 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30319)
(https://github.com/bitcoin/bitcoin/pull/30319)
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650961956)
Dropped the new constant leaving the current behaviour as default, unless an optional `fs::perms` is provided in 7ad5349a2ec
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650961956)
Dropped the new constant leaving the current behaviour as default, unless an optional `fs::perms` is provided in 7ad5349a2ec
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962883)
Thanks, i took this simplification in 7ad5349a2ec
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962883)
Thanks, i took this simplification in 7ad5349a2ec
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962320)
Taken in 7ad5349a2ec
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962320)
Taken in 7ad5349a2ec
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650960724)
Thanks, now done in 7ad5349a2ec
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650960724)
Thanks, now done in 7ad5349a2ec
💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1650969447)
I have addressed this by making `StartScheduledTasks` not use `m_rng`, but just grab a new `FastRandomContext` instead; performance doesn't matter for this function.
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1650969447)
I have addressed this by making `StartScheduledTasks` not use `m_rng`, but just grab a new `FastRandomContext` instead; performance doesn't matter for this function.
👍 brunoerg approved a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135729296)
ACK 42eb7b144c09f0404487f2f5d17ca1a3e91c9365
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135729296)
ACK 42eb7b144c09f0404487f2f5d17ca1a3e91c9365
🚀 fanquake merged a pull request: "ci: add option for running tests without volume"
(https://github.com/bitcoin/bitcoin/pull/30310)
(https://github.com/bitcoin/bitcoin/pull/30310)
👍 hebasto approved a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876#pullrequestreview-2135788139)
ACK e3dc64f4990a15df3fd6147831f66fc2a31c71ad, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/29876#pullrequestreview-2135788139)
ACK e3dc64f4990a15df3fd6147831f66fc2a31c71ad, I have reviewed the code and it looks OK.
💬 furszy commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1651026133)
> Any specific reason to generate 1472 outputs when 1471 are picked?
All are picked. The extra output is to cover the fee.
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1651026133)
> Any specific reason to generate 1472 outputs when 1471 are picked?
All are picked. The extra output is to cover the fee.