💬 ozie777 commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2637043316)
> Closes #29552
>
>
>
> Add better descriptions to help string for all binaries. Use format which is correctly detected by help2man when generating manpages.
>
>
>
> Examples:
>
>
>
> Before:
>
>
>
> 
>
>
>
> After:
>
> 
>
>
>
> Demonstration using `bitcoin-cli` also highlights removal o
...
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2637043316)
> Closes #29552
>
>
>
> Add better descriptions to help string for all binaries. Use format which is correctly detected by help2man when generating manpages.
>
>
>
> Examples:
>
>
>
> Before:
>
>
>
> 
>
>
>
> After:
>
> 
>
>
>
> Demonstration using `bitcoin-cli` also highlights removal o
...
💬 ozie777 commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2637048734)
Finalized backup coin-recovery string< {"iv":"HVHHWI+QNsSvhpqAMreMfQ==","v":1,"iter":1000,"ks":128,"ts":64,"mode":"ccm","adata":"","cipher":"aes","salt":"Bki7wWERBQg=","ct":"4IAy8LqYwsQAE0b0Tk/OHGYLaBNVXmUZzucNLA1hXOocRu15lQRMWOYbvbpR73s5EFlkFVoqpxXCLtvGdRs8xacKFzcHAFHAHZ2ZyCl/xoHuMTRu9TmOkBQEco0xKgTRkP7+BhvlbE+XwnO1uF+IXKQE+94Ei5vvKc0yVFDvvDKyGdwlBwyKYpc1YERf3mUWkyEs28C6u7I2LJQNK6XFxiFbmXuUPHLoMHUP3b7mp6Qlj2ctz0UmH82dStr1JFb9RDE0QCifDpk6hecMOuBDXJJnaYg2OEKGmGnhAyspD547fT052CbnPFy4uxfKL86n6D9z8z
...
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2637048734)
Finalized backup coin-recovery string< {"iv":"HVHHWI+QNsSvhpqAMreMfQ==","v":1,"iter":1000,"ks":128,"ts":64,"mode":"ccm","adata":"","cipher":"aes","salt":"Bki7wWERBQg=","ct":"4IAy8LqYwsQAE0b0Tk/OHGYLaBNVXmUZzucNLA1hXOocRu15lQRMWOYbvbpR73s5EFlkFVoqpxXCLtvGdRs8xacKFzcHAFHAHZ2ZyCl/xoHuMTRu9TmOkBQEco0xKgTRkP7+BhvlbE+XwnO1uF+IXKQE+94Ei5vvKc0yVFDvvDKyGdwlBwyKYpc1YERf3mUWkyEs28C6u7I2LJQNK6XFxiFbmXuUPHLoMHUP3b7mp6Qlj2ctz0UmH82dStr1JFb9RDE0QCifDpk6hecMOuBDXJJnaYg2OEKGmGnhAyspD547fT052CbnPFy4uxfKL86n6D9z8z
...
🤔 ozie777 reviewed a pull request: "Update manpage descriptions"
(https://github.com/bitcoin/bitcoin/pull/29686#pullrequestreview-2595998582)
Aoproved
(https://github.com/bitcoin/bitcoin/pull/29686#pullrequestreview-2595998582)
Aoproved
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637053225)
> I think all those benefits could be achieved by adding the glue yet having the developer who wants to test checkout the upstream repo into a subdirectory manually to use it (cmake can complain clearly if missing)
@luke-jr, Yes, we could provide a glue script to download the git repository instead of mirroring it in a git subtree, pinning it to a hash to avoid bringing in unreviewed changes. This is essentially what depends system does now. Do you think a glue script would be better than a s
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637053225)
> I think all those benefits could be achieved by adding the glue yet having the developer who wants to test checkout the upstream repo into a subdirectory manually to use it (cmake can complain clearly if missing)
@luke-jr, Yes, we could provide a glue script to download the git repository instead of mirroring it in a git subtree, pinning it to a hash to avoid bringing in unreviewed changes. This is essentially what depends system does now. Do you think a glue script would be better than a s
...
👍 instagibbs approved a pull request: "test: make sure we are on sync with a peer before checking if they have sent a message"
(https://github.com/bitcoin/bitcoin/pull/31760#pullrequestreview-2596001593)
ACK 3f4b104b1b7e1b87c0be8e395e02b6ae3c5d7b08
Makes sense and can't really hurt to apply it every time.
(https://github.com/bitcoin/bitcoin/pull/31760#pullrequestreview-2596001593)
ACK 3f4b104b1b7e1b87c0be8e395e02b6ae3c5d7b08
Makes sense and can't really hurt to apply it every time.
💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1943081201)
> it returned the same value or very close each time. I did not see any 100% values before reaching the tip.
It makes sense bc your node is not isolated, but when you turn it on before having any peer I think it returns 1.0 and then it goes back to 0.x when it receives some header.
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1943081201)
> it returned the same value or very close each time. I did not see any 100% values before reaching the tip.
It makes sense bc your node is not isolated, but when you turn it on before having any peer I think it returns 1.0 and then it goes back to 0.x when it receives some header.
🤔 rkrux reviewed a pull request: "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases"
(https://github.com/bitcoin/bitcoin/pull/31495#pullrequestreview-2595302398)
Build and migration tests successful at d5e28457a099cd546e757984043f28ba9f6689b1.
Utilising `isMine()` instead of reverse engineering looks to me a cautious approach to take, Concept ACK.
Great PR and enough context to go through for which I will review the PR again.
(https://github.com/bitcoin/bitcoin/pull/31495#pullrequestreview-2595302398)
Build and migration tests successful at d5e28457a099cd546e757984043f28ba9f6689b1.
Utilising `isMine()` instead of reverse engineering looks to me a cautious approach to take, Concept ACK.
Great PR and enough context to go through for which I will review the PR again.
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942646524)
`legacy`
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942646524)
`legacy`
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942794684)
`// Invalid scripts such as P2SH-P2SH and P2WSH-P2SH, among others, will be added as candidates.`
In the below function, I notice script itself, P2SH, P2WSH, P2SH-P2WSH being added only. These invalid scripts could be added as candidates because the underlying `script` could be a P2SH?
With this script nesting involved, it makes me want to read the logic of `IsMine` to see how it handles it!
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942794684)
`// Invalid scripts such as P2SH-P2SH and P2WSH-P2SH, among others, will be added as candidates.`
In the below function, I notice script itself, P2SH, P2WSH, P2SH-P2WSH being added only. These invalid scripts could be added as candidates because the underlying `script` could be a P2SH?
With this script nesting involved, it makes me want to read the logic of `IsMine` to see how it handles it!
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942786866)
`contain`
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942786866)
`contain`
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942841714)
Nit:
```diff
- if (!Assume(spks.size() == 0)) {
+ if (!Assume(spks.empty())) {
```
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942841714)
Nit:
```diff
- if (!Assume(spks.size() == 0)) {
+ if (!Assume(spks.empty())) {
```
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942897067)
`have`
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942897067)
`have`
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942865073)
Thanks for this comment, very helpful!
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942865073)
Thanks for this comment, very helpful!
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942990827)
`self.old_node`?
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1942990827)
`self.old_node`?
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943021410)
Where was the first check?
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943021410)
Where was the first check?
💬 rkrux commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943008904)
Nit: Can add a corresponding call before sending the transaction that checks the balance is 6.
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943008904)
Nit: Can add a corresponding call before sending the transaction that checks the balance is 6.
🤔 sipa reviewed a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2596026595)
utACK e3622a969293feea75cfadc8f7c6083edcd6d8de
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2596026595)
utACK e3622a969293feea75cfadc8f7c6083edcd6d8de
💬 sipa commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1943088145)
It's not very nice to expose this internal value in the interface, if we want it to be stable. What about using the [BIP155 network ID](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki#user-content-Specification) instead?
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1943088145)
It's not very nice to expose this internal value in the interface, if we want it to be stable. What about using the [BIP155 network ID](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki#user-content-Specification) instead?
📝 hebasto opened a pull request: "depends: Avoid using the `-ffile-prefix-map` compiler option"
(https://github.com/bitcoin/bitcoin/pull/31800)
This PR is similar to https://github.com/bitcoin/bitcoin/pull/31337 and applies analogous changes to all dependency packages.
The issue was [recently noticed](https://github.com/bitcoin/bitcoin/pull/31661#discussion_r1923896475) when `-ffile-prefix-map` was added to the `libevent` package, which is built in OSS-Fuzz.
This PR replaces `-ffile-prefix-map` from in packages for consistency.
(https://github.com/bitcoin/bitcoin/pull/31800)
This PR is similar to https://github.com/bitcoin/bitcoin/pull/31337 and applies analogous changes to all dependency packages.
The issue was [recently noticed](https://github.com/bitcoin/bitcoin/pull/31661#discussion_r1923896475) when `-ffile-prefix-map` was added to the `libevent` package, which is built in OSS-Fuzz.
This PR replaces `-ffile-prefix-map` from in packages for consistency.
💬 hebasto commented on issue "fuzz: oss-fuzz coverage build is failing":
(https://github.com/bitcoin/bitcoin/issues/31770#issuecomment-2637098504)
A fix has been proposed in https://github.com/bitcoin/bitcoin/pull/31800.
(https://github.com/bitcoin/bitcoin/issues/31770#issuecomment-2637098504)
A fix has been proposed in https://github.com/bitcoin/bitcoin/pull/31800.