💬 ariard commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1935077559)
> After a few attempts, I haven't found a way to trigger a consensus fork through adjusted time.
Unclear what has been tested actually (e.g setting sampled time of peers in the past and playing with blocks timestamps).
That said I think it’s a very edge scenario, and most susceptible to affect non-upgraded things like btcd.
> I now believe that the network-adjusted time implementation in master does not offer any security guarantees against an attacker trying to get a node out of consensu
...
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1935077559)
> After a few attempts, I haven't found a way to trigger a consensus fork through adjusted time.
Unclear what has been tested actually (e.g setting sampled time of peers in the past and playing with blocks timestamps).
That said I think it’s a very edge scenario, and most susceptible to affect non-upgraded things like btcd.
> I now believe that the network-adjusted time implementation in master does not offer any security guarantees against an attacker trying to get a node out of consensu
...
💬 sdaftuar commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1935097759)
@ariard I agree that this would be simpler if there is just 1 anchor output (see also @instagibbs' proposal for ephemeral anchors, fwiw). But (a) that is not up to this project to decide; (b) you still need some kind of pinning solution if a too-large spend is created (which v3 provides -- while no other workable proposals have been put forward); and (c) even then it sounds like it would take LN some time to coordinate an upgrade to a 1-anchor system (whether EA or 2 branches of a single script
...
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1935097759)
@ariard I agree that this would be simpler if there is just 1 anchor output (see also @instagibbs' proposal for ephemeral anchors, fwiw). But (a) that is not up to this project to decide; (b) you still need some kind of pinning solution if a too-large spend is created (which v3 provides -- while no other workable proposals have been put forward); and (c) even then it sounds like it would take LN some time to coordinate an upgrade to a 1-anchor system (whether EA or 2 branches of a single script
...
💬 achow101 commented on pull request "test: maxuploadtarget: check for mempool msg disconnect if limit is reached, improve existing test coverage":
(https://github.com/bitcoin/bitcoin/pull/28996#issuecomment-1935137249)
ACK b58f009d9585aab775998644de07e27e2a4a8045
(https://github.com/bitcoin/bitcoin/pull/28996#issuecomment-1935137249)
ACK b58f009d9585aab775998644de07e27e2a4a8045
💬 araujo88 commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935143088)
I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
```
- self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
+ self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLn
...
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935143088)
I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
```
- self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
+ self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLn
...
🚀 achow101 merged a pull request: "test: maxuploadtarget: check for mempool msg disconnect if limit is reached, improve existing test coverage"
(https://github.com/bitcoin/bitcoin/pull/28996)
(https://github.com/bitcoin/bitcoin/pull/28996)
📝 pablomartin4btc opened a pull request: "doc: Update translation process guide"
(https://github.com/bitcoin/bitcoin/pull/29414)
Updating Transifex broken link and setup Transifex config file with a token.
(https://github.com/bitcoin/bitcoin/pull/29414)
Updating Transifex broken link and setup Transifex config file with a token.
💬 achow101 commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935265831)
> I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
>
> A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
>
> ```
> - self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
> + self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Po
...
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935265831)
> I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
>
> A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
>
> ```
> - self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
> + self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Po
...
💬 epiccurious commented on pull request "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`":
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1935284702)
If a length of 32 is insufficient, why did you choose 1000 vs 100 or 10,000?
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1935284702)
If a length of 32 is insufficient, why did you choose 1000 vs 100 or 10,000?
💬 epiccurious commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1935287441)
How do you define a mutated block? What are the known forms of mutated blocks?
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1935287441)
How do you define a mutated block? What are the known forms of mutated blocks?
💬 daniel-btc-nym commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1935289038)
Yeah, I ran some valgrind tests before and after my changes and it clearly showed the "uninitialized" accesses when we copy the data first into a tight vector as you suggested.
I added more test coverage and think this should be enough for this issue.
What do you think about [the changes](https://github.com/bitcoin/bitcoin/compare/master...daniel-btc-nym:bitcoin:master)
If they look OK, I can merge them into a single commit -- (I suppose by forking again?) -- and then making a pull
...
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1935289038)
Yeah, I ran some valgrind tests before and after my changes and it clearly showed the "uninitialized" accesses when we copy the data first into a tight vector as you suggested.
I added more test coverage and think this should be enough for this issue.
What do you think about [the changes](https://github.com/bitcoin/bitcoin/compare/master...daniel-btc-nym:bitcoin:master)
If they look OK, I can merge them into a single commit -- (I suppose by forking again?) -- and then making a pull
...
💬 epiccurious commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1935289598)
Concept ACK f4db3546c003822e485f1597701f278eef197619.
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1935289598)
Concept ACK f4db3546c003822e485f1597701f278eef197619.
💬 epiccurious commented on pull request "build: Add missed definition for `AM_OBJCXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29362#issuecomment-1935291945)
utACK 2fb758da581b5cefcc0232e1b40f13143ad2bdaf
(https://github.com/bitcoin/bitcoin/pull/29362#issuecomment-1935291945)
utACK 2fb758da581b5cefcc0232e1b40f13143ad2bdaf
💬 araujo88 commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935314945)
> > I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
> > A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
> > ```
> > - self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
> > + self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3
...
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1935314945)
> > I'm getting an error in one the functional tests cases in `test/functional/rpc_getdescriptorinfo.py`.
> > A fix would be changing the derivation path from `1/2h` (hardened) to `1/2` (non-hardened):
> > ```
> > - self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1h/2)", isrange=False, issolvable=True, hasprivatekeys=False)
> > + self.test_desc("pkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3
...
📝 hernanmarino opened a pull request: "gui: Correct tooltip wording for watch-only wallets"
(https://github.com/bitcoin-core/gui/pull/792)
This is a continuation of https://github.com/bitcoin-core/gui/pull/37
This is a simple change, but imho necessary because it leads to confusion.
The objective of this change is to provide a better description for the tooltip for the "Available Balance" balance because the current version on master mentions the word "You current spendable balance" which is a bit misleading, because that balance is not always spendable , or not always yours on watch only wallets.
(https://github.com/bitcoin-core/gui/pull/792)
This is a continuation of https://github.com/bitcoin-core/gui/pull/37
This is a simple change, but imho necessary because it leads to confusion.
The objective of this change is to provide a better description for the tooltip for the "Available Balance" balance because the current version on master mentions the word "You current spendable balance" which is a bit misleading, because that balance is not always spendable , or not always yours on watch only wallets.
💬 hernanmarino commented on pull request "gui: Correct tooltip wording for watch-only wallets":
(https://github.com/bitcoin-core/gui/pull/792#issuecomment-1935323075)
As a first commit, I only took @ saibato's latest commit and corrected a couple of bugs (mentioned by @pablomartin4btc in the original PR.)
Marked as Draft, in the following days I'll try to address all comments and sugestions in that PR, and mark it ready for review.
(https://github.com/bitcoin-core/gui/pull/792#issuecomment-1935323075)
As a first commit, I only took @ saibato's latest commit and corrected a couple of bugs (mentioned by @pablomartin4btc in the original PR.)
Marked as Draft, in the following days I'll try to address all comments and sugestions in that PR, and mark it ready for review.
💬 hernanmarino commented on pull request "indicate explicit to the user that the wallet balances shown is watch only.":
(https://github.com/bitcoin-core/gui/pull/37#issuecomment-1935326761)
Hi, if no one disagrees, I am taking over the development of this functionality. Discussion can continue here: https://github.com/bitcoin-core/gui/pull/792 Thanks @Saibato for the work
(https://github.com/bitcoin-core/gui/pull/37#issuecomment-1935326761)
Hi, if no one disagrees, I am taking over the development of this functionality. Discussion can continue here: https://github.com/bitcoin-core/gui/pull/792 Thanks @Saibato for the work
🤔 stratospher reviewed a pull request: "rpc: parse legacy pubkeys consistently with specific error messages"
(https://github.com/bitcoin/bitcoin/pull/28336#pullrequestreview-1871661382)
Concept ACK! liked how the error handling is done in a consitent manner now.
(https://github.com/bitcoin/bitcoin/pull/28336#pullrequestreview-1871661382)
Concept ACK! liked how the error handling is done in a consitent manner now.
💬 stratospher commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1483864342)
b8ee41b: is it impossible to have valid bitcoin addresses which are just hex characters?
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1483864342)
b8ee41b: is it impossible to have valid bitcoin addresses which are just hex characters?
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1483975288)
I see thanks for the explanation. It seems to me like most of these cases should be caught by the optimization to cut instead of shift when we exceed the weight:
> opt: Cut if last addition was minimal weight
>
> In situations where we have UTXO groups of various weight, we can CUT
> rather than SHIFT when we exceeded the max_weight or the best
> selection’s weight while the last step was equal to the minimum weight
> in the lookahead.
It would take more time to determine whether th
...
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1483975288)
I see thanks for the explanation. It seems to me like most of these cases should be caught by the optimization to cut instead of shift when we exceed the weight:
> opt: Cut if last addition was minimal weight
>
> In situations where we have UTXO groups of various weight, we can CUT
> rather than SHIFT when we exceeded the max_weight or the best
> selection’s weight while the last step was equal to the minimum weight
> in the lookahead.
It would take more time to determine whether th
...
💬 maflcko commented on pull request "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`":
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1935504931)
lgtm ACK 864e2e9097de8f1fda63137f803687dd5cc96c03
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1935504931)
lgtm ACK 864e2e9097de8f1fda63137f803687dd5cc96c03