💬 theuni commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-1671392361)
@dergoegge Thanks for the ping.

> makes operations such as deleting them from multiple threads difficult

Why is this a goal?

I'm quite nervous about these changes. I've spoken with @dergoegge about this a few times now, but I'm afraid my information and opinions on the code involved are quite dated at this point (2017 to be exact :), so I haven't had much to contribute.

I'll say though, any changes or refactors for future changes to the threading model should be very well justified
...

💬 ryanofsky commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288565564)
In commit "rpc: Add Arg() default helper" (fa56ad74160ba3c07a16a661fc14cded6deed9eb)

Haven't really looked at implementation yet, but can documentation be updated to say what happens with type mismatches? E.g. if arg is an int and you request a bool? Is the value just ignored and is a default returned instead? Is there an exception?

Also might be nice to add a short examples here showing what equivalent low level code is so developers can know how to use this helper and interpret it. Like
...

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1288400379)
Having the managed objects have a reference to the manager leads to absolute spaghetti code, so leaning towards NACK on that.

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1288527915)
You can just get rid of NodesSnapshot entirely now, it was only needed as a RAII helper for the manual ref counting.

💬 SambhavsCreation commented on issue "test: 999 of 999 multisig":
(https://github.com/bitcoin/bitcoin/issues/28179#issuecomment-1671550220)
Is someone still working on this? Please let me know as if not I might start working on it myself

💬 eriknylund commented on issue "test: 999 of 999 multisig":
(https://github.com/bitcoin/bitcoin/issues/28179#issuecomment-1671555701)
> Is someone still working on this? Please let me know as if not I might start working on it myself

I have a PR https://github.com/bitcoin/bitcoin/pull/28212 open for it, so please review if you have a chance. 🙏

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1671573912)
I ended up "borrowing" some code from the followup PRs, without having to rebase on them.

This code compiles and the index doesn't crash. That's about the extend of it though :-) For now.

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-1671589152)
>> makes operations such as deleting them from multiple threads difficult
>
> Why is this a goal?

I missed this in the OP and think that this should not be a goal or should be well justified.

I like getting rid of the custom ref counting for other reasons. Mainly, because it's annoying and bug prone.

Something I have been working on is hiding CNode from the public net interface and turning the connection manager into a NodeId based manager, e.g. `CConnman::PushMessage(CNode*, CSerial
...

💬 willcl-ark commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-1671595104)
> @dergoegge Thanks for the ping.
>
> > makes operations such as deleting them from multiple threads difficult
>
> Why is this a goal?
>
> I'm quite nervous about these changes. I've spoken with @dergoegge about this a few times now, but I'm afraid my information and opinions on the code involved are quite dated at this point (2017 to be exact :), so I haven't had much to contribute.
>
> I'll say though, any changes or refactors for future changes to the threading model should be ver
...

📝 achow101 opened a pull request: "wallet: Use CTxDestination in CRecipient instead of just scriptPubKey"
(https://github.com/bitcoin/bitcoin/pull/28246)
For silent payments, we want to provide a `SilentPaymentsDestination` to be used as the recipient, which requires `CRecipient` to use something other than just the `scriptPubKey` as we cannot know the output script for a silent payment prior to transaction creation. `CTxDestination` seems like the obvious place to add a `SilentPaymentsDestination` as it is our internal representation of an address.

In order to still allow paying to arbitrary scriptPubKeys (e.g. for data carrier outputs, or th
...

⚠️ reclaim4u opened an issue: "BITCOIN RECOVERY"
(https://github.com/bitcoin/bitcoin/issues/28247)
### Motivation

Losing money on the internet is all too common these days, and I don't see why anyone should be ashamed to admit it. The frightening thing is that anyone can become a victim, regardless of intelligence,I would have considered reporting the theft to law enforcement or the FBI's anti-money laundering unit, but the chances of those actions resulting to the recovery of stolen crypto are extremely remote.

### Possible solution

I learnt about the Reclaim RECOVERY CENTRE INC. through
...

💬 reclaim4u commented on pull request "wallet: Use CTxDestination in CRecipient instead of just scriptPubKey":
(https://github.com/bitcoin/bitcoin/pull/28246#issuecomment-1671770517)
I thought I was entering an age of new money from online investment when I learnt about binary options, I was greatly eager to get on board even more especially because i had some knowledge of online trading, everything changed when I was ready to make my withdrawal and I could get nothing. The entire thing literally broke down in my face when I became convinced my money was stuck with them, read came across a recovery experts and they did a good job helping me settle a problem that was having m
...

🤔 mzumsande reviewed a pull request: "doc: diversify network outbounds release note"
(https://github.com/bitcoin/bitcoin/pull/28189#pullrequestreview-1570099879)
ACK 7463d259e14676846f0b43e2b5f2af3faf688cb4

Release note looks good to me!

💬 instagibbs commented on issue "BITCOIN RECOVERY":
(https://github.com/bitcoin/bitcoin/issues/28247#issuecomment-1671800403)
definitely not a scam

✅ fanquake closed an issue: "BITCOIN RECOVERY"
(https://github.com/bitcoin/bitcoin/issues/28247)

:lock: fanquake locked an issue: "BITCOIN RECOVERY"
(https://github.com/bitcoin/bitcoin/issues/28247)

🚀 fanquake merged a pull request: "doc: diversify network outbounds release note"
(https://github.com/bitcoin/bitcoin/pull/28189)

💬 BrandonOdiwuor commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#discussion_r1288952594)
Also consider adding logs to this test as the ones above

💬 BrandonOdiwuor commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#discussion_r1288951022)
Incorporating logs within the test is crucial for offering transparent insight into the test's progression, simplifying the identification of problems, and enhancing comprehension of the test's overall behavior.

💬 TheCharlatan commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1289035093)
> Also, the value will be nullptr before and after the call to HandleRequest

Does this also hold if `m_fun` throws an exception?