< bitcoin-git>
[bitcoin] freenancial closed pull request #19480: build: Only use `@` prefix for `echo` command in Makefiles (master...remove_at_sign_makefile) https://github.com/bitcoin/bitcoin/pull/19480
< bitcoin-git>
[bitcoin] sdaftuar opened pull request #19596: Deduplicate parent txid loop of requested transactions and missing parents of orphan transactions (master...2020-07-dedup-inputs) https://github.com/bitcoin/bitcoin/pull/19596
< sipa>
bitcoin core's internal varint type isn't used in the p2p protocol anywhere
< vasild>
I implemented (de)ser_varint: https://bpa.st/HTBA, it will end up in some of the addrv2 PRs (still I am only playing with it locally)
< sipa>
vasild: could it just mean compactsize ?
< sipa>
that's the varint type that's used elsewhere in the p2p protocol
< vasild>
Isn't used _yet_ ;-)
< vasild>
I agree it is underspecified other than "see bitcoin core source code"
< sipa>
i don't even see that?
< vasild>
no idea if it is supposed to mean compactsize, wumpus ?
< vasild>
"i don't even see that?" -- yes, not even "see bitcoin core source code", but I think that would be undesirable because the code may change
< vasild>
why are there two encodings, compactsize and varint?
< sipa>
compactsize is the one the original source code and protocol from satoshi's time used
< sipa>
which was called varint everywhere, but compactsize inside the source code (as it was only used to serialize the length of things)
< sipa>
the varint one was inspired by BER's length encoding, with a slight (and in retrospect not worthwhile) efficiency improvement i came up with
< sipa>
it was part of the database redesign in 0.8 that introduced the utxo set chainstate database
< sipa>
where i think it performed better than the compactsize we had
< vasild>
I see
< vasild>
is compactsize specified anywhere (other than "look at the code")?
< sipa>
and maybe part of the confusion here is because the compactsize encoding is often jusy referred to as varint
< sipa>
vasild: not in any BIPs afaik; things that existed in the protocol before the BIPs process existed as generally just taken as gospel
< vasild>
maybe the new type should have been named something else than varint
< vasild>
Ok, so neither one of varint or compactsize is defined in BIPs, but compact size is already used in other places in the p2p protocol whereas varint is not.
< vasild>
BIP155 says:
< vasild>
- Jonas Schnelli: change services field to VARINT, to make the message more compact in the likely case instead of always using 8 bytes.
< vasild>
- Luke-Jr: change time field to VARINT, for post-2038 compatibility.
< vasild>
jonasschnelli: luke-jr: ^^^ Did you mean compactsize instead of varint? Should we change that to compactsize now?
< sipa>
for BIP152 there was initially an intention to use varint, but it was changed to compactsize to simplify specification
< jonasschnelli>
what is the difference between compact size and varint?
< sipa>
vasild: ^ that's what i mean :p
< vasild>
Or, well, if in other specs the word "VARINT" is used to mean "compactsize", then should the BIP155 C++ source code use compactsize?
< sipa>
when people say varint in the context of the protocol, it usually refers to what is known as compactsize in the source code
< sipa>
jonasschnelli: scroll up a few lines, i gave the history :)
< vasild>
and "compactsize" is mentioned in 6 BIPs
< sipa>
which ones?
< vasild>
varint:
< vasild>
bip-0010.mediawiki
< vasild>
bip-0023.mediawiki
< vasild>
bip-0037.mediawiki
< vasild>
bip-0098.mediawiki
< vasild>
bip-0154.mediawiki
< vasild>
bip-0155.mediawiki
< vasild>
bip-0180.mediawiki
< vasild>
bip-0322.mediawiki
< vasild>
compactsize:
< vasild>
bip-0152.mediawiki
< vasild>
bip-0157.mediawiki
< vasild>
bip-0158.mediawiki
< vasild>
bip-0330.mediawiki
< vasild>
bip-0341.mediawiki
< vasild>
bip-0342.mediawiki
< vasild>
Anyway, given that compact size is used in p2p and not varint, I suggest that we change the spec to say "compactsize" instead of "varint" with some clarification like "For the purposes of this section, CompactSize refers to the variable-length integer encoding used acros..." (from bip152) and also change the code to use compactsize instead of varint (the code is in
< luke-jr>
I'm not sure it makes sense to use the old varint for time/services.. benefits more from the new varint
< vasild>
luke-jr: actually 4 bytes unsigned integer supports time up to year 2106. Only signed 4 bytes will brick after 2038
< vasild>
CAddress::nTime is uint32_t - it will work as is in master until 2106. I think there is no need to change it to varint or compact size in order to make it post-2038 proof. varint or compactsize would need 5 bytes today.
< vasild>
For services - both varint and compact size will use 1 byte if NODE_NETWORK_LIMITED is not involved.
< vasild>
And if NODE_NETWORK_LIMITED is set, then compactsize will use 3 bytes whereas varint will use 2 bytes.
< bitcoin-git>
[bitcoin] theStack opened pull request #19597: test: test decodepsbt fee calculation (count input value only once per UTXO) (master...20200726-test-check-deceodepsbt-fee-calculation) https://github.com/bitcoin/bitcoin/pull/19597
< vasild>
Is saving 1 byte worth introducing a new encoding?
< wumpus>
vasild: does it introduce a new encoding?
< wumpus>
saving a byte per address sounds good actually
< vasild>
wumpus: as it turns out VARINT is not used anywhere in P2P, CompactSize is used instead.
< wumpus>
ok
< vasild>
So from P2P point of view VARINT is a new encoding.
< wumpus>
so VARINT is only used in on-disk files?
< wumpus>
agree that it'd be preferable not to introduce it into P2P then
< wumpus>
though it's not a complex encoding
< vasild>
btw, I did not check that myself, but I trust sipa on it
< vasild>
both varint and compactsize are mentioned in some BIPs
< vasild>
but I gather "varint" in BIPs may be used to denote what the source code calls compactsize
< wumpus>
I'm definitely going with sipa's judgement on this :)
< vasild>
yes, it is not complex - I did the python implementation in order to do some more testing of it, python ser/deser: https://bpa.st/HTBA
< vasild>
btw, there is one more concern wrt VARINT - it supports unlimited size and the deserializer would throw an exception for a code like: uint64_t x; stream >> VARINT(x); given a specially crafted sequence of bytes
< vasild>
e.g. 9 bytes that all have the first bit set would overflow uint64_t
< vasild>
So, any code that reads varint from the network has to do: try { stream >> VARINT(x); } catch (...) { aha! }
< sipa>
ahmed_: most likely that means you sent a message that's shorter than expected
< ahmed_>
@sipa thats why im confused, ive pulled CAddress code from @petertodd's bitcoin-python and the Version message code is tidied up version of the msg_version code in his repo
< bitcoin-git>
[bitcoin] JeremyRubin opened pull request #19601: Refactoring CHashWriter & Get{Prevouts,Sequence,Outputs}Hash to SHA256 (Alternative to #18071) (master...refactoring-hashers-2) https://github.com/bitcoin/bitcoin/pull/19601
< bitcoin-git>
[bitcoin] achow101 opened pull request #19602: wallet: Migrate legacy wallets to descriptor wallets (master...descriptor-wallet-migration) https://github.com/bitcoin/bitcoin/pull/19602