💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838271193)
Yeah old language, will touch in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838271193)
Yeah old language, will touch in follow-up
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838272598)
no reason, leaving as is
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838272598)
no reason, leaving as is
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838273861)
good question, I'll take a look on follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838273861)
good question, I'll take a look on follow-up
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838275419)
used to have a use, let me look at it in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838275419)
used to have a use, let me look at it in follow-up
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838277119)
will take a look in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838277119)
will take a look in follow-up
💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>{8}` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2470809178)
Thanks @fanquake, I thought of that, can you please help me understand the constraints?
Wouldn't that require a cmake generation step from binary to header which would basically produce the exact same lines as what we have now?
Would it help if I simply extracted it to a separate header file instead?
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2470809178)
Thanks @fanquake, I thought of that, can you please help me understand the constraints?
Wouldn't that require a cmake generation step from binary to header which would basically produce the exact same lines as what we have now?
Would it help if I simply extracted it to a separate header file instead?
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838287609)
the version doesn't really matter since we set `mempool_opts.min_relay_feerate = CFeeRate(0);`
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838287609)
the version doesn't really matter since we set `mempool_opts.min_relay_feerate = CFeeRate(0);`
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838289018)
will look in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838289018)
will look in follow-up
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470819890)
compiling a list of things to do in follow-up, otherwise keeping as-is
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470819890)
compiling a list of things to do in follow-up, otherwise keeping as-is
💬 furszy commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1838292730)
> Out of curiosity: Do you think we should limit printing the data here to a certain length or printing all of it is fine as well? Idk atm if the RPC responses from core have a size limit.
Should check the limit, there shouldn't be any. But, as this is an error message, which shouldn't usually happen, I think that it is better to receive the entire content.
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1838292730)
> Out of curiosity: Do you think we should limit printing the data here to a certain length or printing all of it is fine as well? Idk atm if the RPC responses from core have a size limit.
Should check the limit, there shouldn't be any. But, as this is an error message, which shouldn't usually happen, I think that it is better to receive the entire content.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838292789)
will take a look in followup
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838292789)
will take a look in followup
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838294911)
will look in followup
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838294911)
will look in followup
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838295658)
will look in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838295658)
will look in follow-up
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838296688)
lots of code history blown away I'm sure :)
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838296688)
lots of code history blown away I'm sure :)
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838302730)
The same argument should be made for not allowing modified fees too right? There's an argument to be made, but I think it's more expansive than just the RPC.
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838302730)
The same argument should be made for not allowing modified fees too right? There's an argument to be made, but I think it's more expansive than just the RPC.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838305310)
will add in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838305310)
will add in follow-up
🤔 marcofleon reviewed a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2429970052)
Tested ACK 9c5775c331e02dab06c78ecbb58488542d16dda7. Reproduced the crash on master and checked that this fixed it. The checks added to `GetAddr_` look reasonable.
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2429970052)
Tested ACK 9c5775c331e02dab06c78ecbb58488542d16dda7. Reproduced the crash on master and checked that this fixed it. The checks added to `GetAddr_` look reasonable.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470893774)
addrman test failure for win64, assuming unrelated: https://github.com/bitcoin/bitcoin/pull/30239/checks?check_run_id=32868463159
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470893774)
addrman test failure for win64, assuming unrelated: https://github.com/bitcoin/bitcoin/pull/30239/checks?check_run_id=32868463159
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838351237)
will take a crack at it in follow-up
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838351237)
will take a crack at it in follow-up
💬 fanquake commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470949674)
> addrman test failure for win64, assuming unrelated:
Yea this is a Wine issue (see #31071 & the linked issues). I restarted the job.
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2470949674)
> addrman test failure for win64, assuming unrelated:
Yea this is a Wine issue (see #31071 & the linked issues). I restarted the job.