<michaelfolkson>
How did it get around the libevent stuff?
<sipa>
It doesn't use libevent.
<michaelfolkson>
What does it do instead?
<michaelfolkson>
So at least until UNIX sockets are implemented in Core non-multiprocess that's a reason for using multiprocess? If you want to use UNIX sockets?
<sipa>
I can't understand your reasoning.
<sipa>
They do different things.
<sipa>
You don't "want" to use UNIX sockets. They're a tool to accomplish a goal.
<sipa>
Using multiprocess doesn't let you make RPC connections over UNIX sockets.
<sipa>
Because multiprocess isn't RPC.
<earnestly>
capn proto is rpc
<sipa>
Yes, multiprocess uses capnproto under the hood/
<sipa>
But it doesn't (and can't) provide the Bitcoin Core RPC interface. It's used to provide the multiprocess interface.
<sipa>
P2P also doesn't use libevent; it uses raw socket. It also doesn't provide the RPC interface (nor the multiprocess interface); it provides the P2P interface.
<sipa>
If you're using Linux, and using bitcoin-qt, Qt is likely for some part at least using a UNIX domain socket to communicate with the X graphics driver. So if your goal was "I want my Bitcoin Core to use UNIX domain sockets!!!", then you probably already have it.
<MarcoFalke>
why would using sockets be a goal by itself?
<sipa>
It's not. That was my point - you don't care about using UNIX domain sockets on itself.
<MarcoFalke>
right. It shouldn't matter too much to the user what multiprocess uses internally to communicate
<sipa>
RPC, P2P, multiprocess, UI... they're all distinct ways in which Bitcoin Core interacts with other processes/systems; ... and they all have different needs, and use different tools to accomplish that. Just because one uses a particular technology under the hood doesn't mean it's easy to make another one use the same technology.
<michaelfolkson>
I was thinking (incorrectly) that because multiprocess uses UNIX sockets that meant there was UNIX socket support for the equivalent of multiprocess RPC
<michaelfolkson>
Thanks for the explanation
vasild has joined #bitcoin-core-dev
bomb-on has quit [Quit: aллилѹіа!]
grettke has joined #bitcoin-core-dev
jespada has quit [Ping timeout: 256 seconds]
grettke has quit [Read error: Connection reset by peer]
jespada has joined #bitcoin-core-dev
bomb-on has joined #bitcoin-core-dev
arythmetic has joined #bitcoin-core-dev
x88x88x has quit [Ping timeout: 240 seconds]
jarthur has joined #bitcoin-core-dev
grettke has joined #bitcoin-core-dev
x88x88x has joined #bitcoin-core-dev
grettke has quit [Read error: Connection reset by peer]
andrewtoth has quit [Remote host closed the connection]
erik1 has joined #bitcoin-core-dev
Guest97 has quit [Quit: Client closed]
kexkey has joined #bitcoin-core-dev
<jamesob>
Okay, it seems something has changed with how -maxtipage is handled... I wonder if any of the commandline args handling stuff has changed (in this case `args.GetIntArg`)? Because I'm passing `-maxtipage=999999999999` and by the time we hit IsIBD, its value is 0
<jamesob>
This wasn't the case last time I did assumeutxo functional testing a month or two ago
vasild has quit [Remote host closed the connection]
vasild has joined #bitcoin-core-dev
bitdex has quit [Remote host closed the connection]
bitdex has joined #bitcoin-core-dev
arythmetic has quit [Remote host closed the connection]
geyaeb has quit [Remote host closed the connection]
<jamesob>
Ah yep, that was it. The value I'd been passing in the test script was too large for int64; old code I guess interpreted it as `std::numeric_limits<uint64_t>::max()`, new code gives 0. Little concerning that we don't warn on the incompatibility
geyaeb has joined #bitcoin-core-dev
arythmetic has joined #bitcoin-core-dev
bitdex has quit [Remote host closed the connection]
bitdex has joined #bitcoin-core-dev
x88x88x has quit [Ping timeout: 240 seconds]
tla2k21 has quit [Remote host closed the connection]
tla2k21 has joined #bitcoin-core-dev
jb55 has joined #bitcoin-core-dev
<MarcoFalke>
jamesob: Integer overflow inside atoi is UB or at least unspecified behavior, last time I checked
x88x88x has joined #bitcoin-core-dev
<MarcoFalke>
That is: Parsing 999999999999 can give you any value (it doesn't have to be 0 or max or 999999999999 mod max)
<MarcoFalke>
jamesob: It is also not used in consensus code, so this is not a consensus change
<jamesob>
so here's the tricky thing: our atoi64 wasn't actually using atoi, but strtoll - for which oveflow behavior is well defined: "The strtol() function returns the result of the conversion, unless the value would underflow or overflow. If an underflow occurs, strtol() returns LONG_MIN. If an overflow occurs, strtol() returns LONG_MAX."
<jamesob>
("The strtoll() function works just like the strtol() function but returns a long long integer value.")
<MarcoFalke>
Yeah, not consensus. Could be used by bitcoin-util or some RPC function.
<jamesob>
Phew
<MarcoFalke>
Oh, I wasn't aware it is safe and saturating to produce an overflow. Still, an explicit error would be better than silent saturation.
<jamesob>
Right... I think we should implement either full backwards compatibility to strtoll or an explicit error
<luke-jr>
idk, being able to put 9999999999999 is convenient :p
<luke-jr>
depends on the context it's used I suppose
<jamesob>
luke-jr: I certainly thought so haha
<jamesob>
but I mean the point is that IMO (even if it's a minor issue) we shouldn't change that behavior implicitly
<sipa>
It was an unintended behavior change; such things should be caught during review.
<sipa>
Whether the change is desirable or not, it shouldn't be made unintentionally.
<MarcoFalke>
for bitcoin-tx it was broken either way (with or without overflow), already fixed in #23227
<MarcoFalke>
I think arg parsing is the only place remaining that is affected by odd behavior
<MarcoFalke>
If saturation is something that people rely on, we should have a test for this
<MarcoFalke>
*should have had
<MarcoFalke>
For a quick fix I expect saturation to be easier to re-add. Still, (long term) I'd prefer InitError on startup
<jamesob>
I'll add saturation in the PR
<jamesob>
Will also create an issue to surface InitErrors if we don't already have one
<luke-jr>
MarcoFalke: kinda hard to remember maxint64's value tho; maybe if we also parse "inf" or something? :/
<luke-jr>
or "max" might be more correct
<jamesob>
luke-jr: I think just maintaining strict compatbility with the old method (strtoll) is probably the way to go
<MarcoFalke>
jamesob: If you add saturation, please also add a test ;)
<jamesob>
MarcoFalke: sure; though technically the one I already wrote should cover it, but agree that's maybe too implicit
<MarcoFalke>
luke-jr: I understand why you don't want to rebase this and I support your choice, but practically speaking it takes less than a few seconds for you to rebase. However, modifying the merge script will take at least a few minutes. So picking the way of least resistance should be trivial here.
<jamesob>
MarcoFalke: luke-jr: I think I'm missing context here... talking about same issue?
<MarcoFalke>
jamesob: Right, should be enough if you add one more case for negative overflow
<MarcoFalke>
jamesob: It is a separate thread
<jamesob>
gotcha
<MarcoFalke>
(didn't want to spam github for it)
x88x88x has quit [Ping timeout: 260 seconds]
x88x88x has joined #bitcoin-core-dev
<sipa>
ha, i didn't know this: cirrus has a big bitcoin core logo on their site under "Trusted by Open Source": https://cirrus-ci.org/
bomb-on has quit [Quit: aллилѹіа!]
<luke-jr>
MarcoFalke: perhaps, but shouldn't the merge script get fixed anyway?
<luke-jr>
what's very curious there, is that my PR doesn't even *touch* the "conflicted" file
bomb-on has joined #bitcoin-core-dev
<MarcoFalke>
Yeah, it is a GitHub bug, but they are not interested in fixing it
<luke-jr>
so I'm really at a loss for what's going on there
<MarcoFalke>
I am not going to fix the merge script, maybe someone else?