< bitcoin-git>
[bitcoin] jtimon opened pull request #11426: BIP90: Make buried deployments slightly more easily extensible (master...e16-bip90-extensible) https://github.com/bitcoin/bitcoin/pull/11426
< jtimon>
re https://github.com/bitcoin/bitcoin/pull/11398 I wonder if we should always leave at least the last bip9/bip8 deployment there to make sure we're using the rpc part of bip9
< jtimon>
I mean, I was about to start the same, but only for csv
< jl2012>
jtimon: I'm trying to make IsSoftForkEnabled(), so all softforks, buried or bip9, could use that
< jl2012>
not sure if it is a good idea
< jl2012>
So next time when we bury a bip9 softfork, we don't need to edit validation.cpp at all
< jtimon>
jl2012: I saw some simplification/preparation on rpc that looked spot on at a first glance, but will review more
< jtimon>
IIRC you were unifying SoftForkMajorityDesc and SoftForkDesc and preparing it for post-bip9 buried deployments
< jl2012>
I'm trying to combine softforks and bip9_softforks in getblockchaininfo
< jtimon>
that kind of thing should pass all the tests before moving anything from bip9 to buried
< jl2012>
ok. Do you think it should be a separate PR?
< jtimon>
yeah, I think leaving BIP9SoftForkDesc as it is and rewritting SoftForkMajorityDesc/SoftForkDesc as it fits as you were doing looks good
< jtimon>
I think most people won't care about them being separated PRs, I slightly care and maybe some people care in the opposite direction (but they can always ignore the dependency PR and ack the upper one directly)
< jtimon>
just eager to ack the csv part I guess, but probably better to focus on commits than PRs
< jtimon>
I would focus first on a commit that leaves everything prepared on the rpc side but without actually changing anything and thus passing all tests, but just my very opinionated and also criticized modus operandi, don't feel obliged to comply
< jl2012>
jtimon: i think you are right
< jtimon>
I think it will make things easier for you, but just try modifying your thing with an interactive rebase and if you're not convinced, rebase --abort (sorry, being verbose about my customes again)
< jtimon>
forget about a separate PR for now, more separated commits will probably do it even for me, the terror of history bike-shedding
< jtimon>
always remember I may compain about tiny things and there the right answer is probably "thanks, but no" I won't be offended
< jl2012>
it's ok
< jtimon>
cool, happy to help, but also don't want to slow you down
< jl2012>
jtimon: the compiler complains comparing uint with int
< jtimon>
cast ?
< jtimon>
(unint32_t) somewhere probably
< jl2012>
i think you changed buried_deployments from int to uint?
< jtimon>
I should have thought about those 2 things before asking you to rebase on top of it, sorry...excitement...impacience...sw is activated!
< luke-jr>
speaking of comparing signed vs unsigned, is it well-defined how it behaves? does it actually compare correctly, or is the warning because of some real bug risk?
< esotericnonsense>
luke-jr: -1 > 1
< esotericnonsense>
my understanding (could be wrong) is that the signed int is cast to unsigned and then compared, so it is well defined, but whether it compares correctly depends on what you mean by correctly :P
< jtimon>
perhaps someone should decide between 64 and 32 (which seems to dominate in consensus code, see primitives/block/CBlockHeader) and do a univeral transparent wrapper for int and unsigned or something
< esotericnonsense>
I don't even get a compiler warning, heh
< jtimon>
yeah, unsigned vs signed is much more dangerous than 32 vs 64
< luke-jr>
esotericnonsense: eck
< luke-jr>
jtimon: since everyone uses 64-bit platforms now, IMO we should make height be uint64_t everywhere
< jtimon>
but in this case I think it should be ok
< luke-jr>
unless we want to support negative heights some places, in which case int64_t is prob fine too
< jtimon>
luke-jr: yeah, makes sense. besides is more forward compatible, we don't want to slow down the rest for miliseconds of performance in inferior platforms, right? I bet they simulate 64 just fine
< luke-jr>
not sure 32-bit is slower on 64-bit platforms, but I don't think we need to support 32-bit performantly.
< jtimon>
jl2012: changed from 32 to 64, mainteined the change to unsigned
< sipa>
converting from signed to unsigned is always well-defined, unsigned to signed is only well-defined if there is no overflow
< jtimon>
no, I was saying probably 64 is slower in 32 platforms, but...not so much slower, that was early optimization
< jtimon>
sipa: thus converting unsigned to signed is never well defined without knowing the input, right?
< sipa>
indeed
< sipa>
(in practice, it works fine, though)
< sipa>
on all platforms we support
< sipa>
64-bit arithmetic is several times slower than 32-bit arithmetic on 32-bit platforms
< sipa>
on 64-bit they're the same speed
< jtimon>
yep, we're discussing edge cases that you just want to be sure about because...consensus code, no? training neural networks this kind of undefined behaviour could be a feature!
< jtimon>
or if not, you probably don't care, whatever the machine does, if the network is not fit for that problem and architecture...just select another one, weights are extremly unlikely to ever get anywhere close to where that matters anyway