💬 parsaei889 commented on pull request "Ephemeral Anchors":
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200630235)
ندارم
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200630235)
ندارم
💬 parsaei889 commented on pull request "Ephemeral Anchors":
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200630532)
عالی
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200630532)
عالی
💬 parsaei889 commented on pull request "Ephemeral Anchors":
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200631415)
خوب بود ممنون
(https://github.com/bitcoin/bitcoin/pull/29001#issuecomment-2200631415)
خوب بود ممنون
📝 fanquake locked a pull request: "Ephemeral Anchors"
(https://github.com/bitcoin/bitcoin/pull/29001)
Depends on https://github.com/bitcoin/bitcoin/pull/28948 and https://github.com/bitcoin/bitcoin/pull/28984
Replaces https://github.com/bitcoin/bitcoin/pull/26403 to refresh the conversation.
BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki
Example usage:
https://github.com/instagibbs/bolts/commits/zero_fee_commitment
https://github.com/instagibbs/lightning/commits/commit_zero_fees
TODO:
1) figure out what precisely to do in a r
...
(https://github.com/bitcoin/bitcoin/pull/29001)
Depends on https://github.com/bitcoin/bitcoin/pull/28948 and https://github.com/bitcoin/bitcoin/pull/28984
Replaces https://github.com/bitcoin/bitcoin/pull/26403 to refresh the conversation.
BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki
Example usage:
https://github.com/instagibbs/bolts/commits/zero_fee_commitment
https://github.com/instagibbs/lightning/commits/commit_zero_fees
TODO:
1) figure out what precisely to do in a r
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661325008)
Yes, it will return something other than `MempoolAcceptResult::ResultType::VALID`. Would happen as well if a conflicting transaction is mined in the meantime.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661325008)
Yes, it will return something other than `MempoolAcceptResult::ResultType::VALID`. Would happen as well if a conflicting transaction is mined in the meantime.
💬 pinheadmz commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2200639046)
@vasild is it possible that in the intermittent case, some other test running in parallel has a process is listening on port 60000? So we connect OK but fail the i2p handshake?
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2200639046)
@vasild is it possible that in the intermittent case, some other test running in parallel has a process is listening on port 60000? So we connect OK but fail the i2p handshake?
💬 mzumsande commented on issue "ci: failure in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200639857)
Looks to me like this is an actual bug that could happen in the real network when making an automatic outbound connection to yourself (where the `opencon` thread instead of a `httpworker` thread opens the connection, whereas the `net` thread accepts connection in both cases), so it's probably not just a ci/test failure?!
Maybe a workable fix would be to take the `PushNodeVersion` call out of `InitializeNode` and call it separately after having added the node to `m_nodes` (which would mean add
...
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200639857)
Looks to me like this is an actual bug that could happen in the real network when making an automatic outbound connection to yourself (where the `opencon` thread instead of a `httpworker` thread opens the connection, whereas the `net` thread accepts connection in both cases), so it's probably not just a ci/test failure?!
Maybe a workable fix would be to take the `PushNodeVersion` call out of `InitializeNode` and call it separately after having added the node to `m_nodes` (which would mean add
...
💬 brunoerg commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2200659833)
> @vasild is it possible that in the intermittent case, some other test running in parallel has a process that is listening on port 60000? So we connect OK but fail the i2p handshake?
Yes, it's possible. I just started a server with `python3 -m http.server 60000` and ran the test (from master branch), I got:
```
AssertionError: [node 0] Expected messages "['Error connecting to h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not parti
...
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2200659833)
> @vasild is it possible that in the intermittent case, some other test running in parallel has a process that is listening on port 60000? So we connect OK but fail the i2p handshake?
Yes, it's possible. I just started a server with `python3 -m http.server 60000` and ran the test (from master branch), I got:
```
AssertionError: [node 0] Expected messages "['Error connecting to h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not parti
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661339294)
There is some layer separation between `net`/`CConnman` and `net_processing`/`PeerManager` which I am trying not to blur too much. The lower `net` level only knows about connections and things like "I need to open N connections of type X". It does not know anything about transactions, or P2P messages which are handled by the `net_processing` layer.
The constant `NUM_PRIVATE_BROADCAST_PER_TX` (_send a transaction to this many peers_) belongs to the `net_processing` layer. Calling this `NumToOp
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661339294)
There is some layer separation between `net`/`CConnman` and `net_processing`/`PeerManager` which I am trying not to blur too much. The lower `net` level only knows about connections and things like "I need to open N connections of type X". It does not know anything about transactions, or P2P messages which are handled by the `net_processing` layer.
The constant `NUM_PRIVATE_BROADCAST_PER_TX` (_send a transaction to this many peers_) belongs to the `net_processing` layer. Calling this `NumToOp
...
💬 brunoerg commented on pull request "fuzz: fix ciphertext size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2200709776)
CI failure is unrelated to this PR. It's the same from https://github.com/bitcoin/bitcoin/issues/30368
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2200709776)
CI failure is unrelated to this PR. It's the same from https://github.com/bitcoin/bitcoin/issues/30368
💬 andrewtoth commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2200722461)
I have the same comment from #28945: Why can we not revert https://github.com/bitcoin/bitcoin/commit/5e4ac5abf54f8e6d6330df0c73119aa0cca4c103 to speed this up instead? I'm not clear why we need to have the `coinsCache` free up all associated memory to use the `PoolAllocator`.
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2200722461)
I have the same comment from #28945: Why can we not revert https://github.com/bitcoin/bitcoin/commit/5e4ac5abf54f8e6d6330df0c73119aa0cca4c103 to speed this up instead? I'm not clear why we need to have the `coinsCache` free up all associated memory to use the `PoolAllocator`.
💬 ryanofsky commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2200723434)
Thanks AJ, it's great to get specific and find out what actual problems you are concerned about, before coming to the conclusion that it is too difficult to remove one hard dependency on global shared state, and therefore we should resort to workarounds such as running code in multiple processes, or finding ways to reset the state and coordinate access to it.
> how do we add temporary logging for development in kernel functions that don't have a manager object handy?
The same way you add t
...
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2200723434)
Thanks AJ, it's great to get specific and find out what actual problems you are concerned about, before coming to the conclusion that it is too difficult to remove one hard dependency on global shared state, and therefore we should resort to workarounds such as running code in multiple processes, or finding ways to reset the state and coordinate access to it.
> how do we add temporary logging for development in kernel functions that don't have a manager object handy?
The same way you add t
...
💬 ryanofsky commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2200737355)
Code review ACK a05f4ca241045e8a4b295760590805f961c2e5e7.
Just changed psbt error string and fixed a test since the last review.
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2200737355)
Code review ACK a05f4ca241045e8a4b295760590805f961c2e5e7.
Just changed psbt error string and fixed a test since the last review.
👍 pinheadmz approved a pull request: "Make it possible to disable Tor binds and abort startup on bind failure"
(https://github.com/bitcoin/bitcoin/pull/22729#pullrequestreview-2152010530)
ACK ffe9b274984a351716348a6c99df19c391bfdc8e
Re-reviewed changes since last ACK, including addition of `tor_port()` in tests which I like.
Also ran my extra test again from https://gist.github.com/pinheadmz/0bea4bf8d8bf9af85f8ae326286d3082
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK ffe9b274984a351716348a6c99df19c391bfdc8e
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaC8jkACgkQ5+KYS2KJ
yT
...
(https://github.com/bitcoin/bitcoin/pull/22729#pullrequestreview-2152010530)
ACK ffe9b274984a351716348a6c99df19c391bfdc8e
Re-reviewed changes since last ACK, including addition of `tor_port()` in tests which I like.
Also ran my extra test again from https://gist.github.com/pinheadmz/0bea4bf8d8bf9af85f8ae326286d3082
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK ffe9b274984a351716348a6c99df19c391bfdc8e
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaC8jkACgkQ5+KYS2KJ
yT
...
💬 willcl-ark commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-2200759744)
Concept ACK.
I was tacking this today (missed this PR as GH search for "sse" yielded no results 🤦🏼♂️ )
First I tried re-implementing @luke-jr previous approach, but refactoring into macros: https://github.com/bitcoin/bitcoin/commit/cdaa9bfd6e63dc675cdd27e6d586f69423ee1748
Whilst this works well-enough, it didn't particularly fix https://github.com/bitcoin/bitcoin/issues/28864 in my opinion, so I tried https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:detect-sse4.
...
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-2200759744)
Concept ACK.
I was tacking this today (missed this PR as GH search for "sse" yielded no results 🤦🏼♂️ )
First I tried re-implementing @luke-jr previous approach, but refactoring into macros: https://github.com/bitcoin/bitcoin/commit/cdaa9bfd6e63dc675cdd27e6d586f69423ee1748
Whilst this works well-enough, it didn't particularly fix https://github.com/bitcoin/bitcoin/issues/28864 in my opinion, so I tried https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:detect-sse4.
...
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661399034)
> Other than that, what if instead of creating this new `LegacyDataSPKM::AddKeyPubKeyInner` method you place this three lines inside the `LegacyDataSPKM::LoadKey` which is only called during load?
We can't do that because `LoadKey` needs to call `LegacyScriptPubKeyMan::AddKeyPubKeyInner` for legacy wallets that are being loaded normally. Since migration cannot do all of the things in `LegacyScriptPubKeyMan::AddKeyPubKeyInner`, we need to have `LegacyDataSPKM::AddKeyPubKeyInner`.
> I think
...
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661399034)
> Other than that, what if instead of creating this new `LegacyDataSPKM::AddKeyPubKeyInner` method you place this three lines inside the `LegacyDataSPKM::LoadKey` which is only called during load?
We can't do that because `LoadKey` needs to call `LegacyScriptPubKeyMan::AddKeyPubKeyInner` for legacy wallets that are being loaded normally. Since migration cannot do all of the things in `LegacyScriptPubKeyMan::AddKeyPubKeyInner`, we need to have `LegacyDataSPKM::AddKeyPubKeyInner`.
> I think
...
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661402030)
Done
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661402030)
Done
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661402170)
Removed the assert and the comment.
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1661402170)
Removed the assert and the comment.
💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#issuecomment-2200765833)
Reordered the commits and preserved the `IsMine` asserts.
(https://github.com/bitcoin/bitcoin/pull/26596#issuecomment-2200765833)
Reordered the commits and preserved the `IsMine` asserts.
💬 mzumsande commented on issue "ci: failure in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200786670)
since the failures are frequent and it seems that the solution won't be all that trivial, I think it might be best to revert https://github.com/bitcoin/bitcoin/pull/30362 temporarily and re-introduce it together with a fix in `net` - without any pressure to get the CI green quickly.
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200786670)
since the failures are frequent and it seems that the solution won't be all that trivial, I think it might be best to revert https://github.com/bitcoin/bitcoin/pull/30362 temporarily and re-introduce it together with a fix in `net` - without any pressure to get the CI green quickly.