Show XTHIN in GUI #8583

Merged
merged 1 commit into from Aug 26, 2016

Projects

None yet

7 participants

@rebroad
Contributor
rebroad commented Aug 25, 2016 edited

Currently GETUTXO is shown in the GUI even though it is not used by Bitcoin Core, but XTHIN is not. Unaware of a reason for this, so therefore this pull request.

Similar to #5876

@luke-jr
Member
luke-jr commented Aug 25, 2016

FWIW, this is in Knots 0.13.0: 2da1d28

@jonasschnelli
Member

utACK 4c3e2cb

Am I right? There is no BIP for XTHIN?
IMO its highly recommended to link the section comments in protocol.h to some specification papers.

@jonasschnelli jonasschnelli added the GUI label Aug 25, 2016
@rebroad
Contributor
rebroad commented Aug 25, 2016

@jonasschnelli #5876 makes no reference to a BIP either. Good idea though. Are there BIPS for GETUTXO and XTHIN? As far as I know there's a "BUIP" for XTHIN but not a BIP, but given XTHIN is being used by Classic, Unlimited, Bitcoin XT, to name a few I think it's somewhat redundant (the BIP) now.

@jonasschnelli
Member

GETUTXO is described in BIP 64.
The only think I could find for XTHIN is https://bitco.in/forum/threads/buip010-passed-xtreme-thinblocks.774/

Not sure if we should add display support if there is no BIP available.

@gmaxwell
Member
gmaxwell commented Aug 25, 2016 edited

Give it six months to see if it even exists on the network then. I'm somewhat doubtful it will. (it's not like this display does anything actually useful in any case)

@laanwj
Member
laanwj commented Aug 25, 2016

No strong opinion about whether to add this or not, I don't think it really hurts. It's not like the bit can be used for anything else at the moment. If it dies out in six months, it can be removed again.

@sipa
Member
sipa commented Aug 25, 2016
@gmaxwell
Member

thats true too, mine was a 'meh, don't bother' not a 'no dont'.

@laanwj
Member
laanwj commented Aug 25, 2016

Yes, I agree it's a waste of time

@jonasschnelli
Member

NACK from my side.
I think we should not reserve a service bit for a feature that is available on 19 "good" node (just checked my seeder):

user:~$ cat dnsseed.dump | grep 00000017 | grep "    1   " | wc -l
19
@MarcoFalke
Member

There appears to be a "detailed protocol specification" which is basically the cpp code copied from the implementation after adding some section headings.

@MarcoFalke
Member

utACK 4c3e2cb, but I also agree with @jonasschnelli

@sipa
Member
sipa commented Aug 25, 2016

Code review ACK.

@laanwj laanwj merged commit 4c3e2cb into bitcoin:master Aug 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laanwj laanwj added a commit that referenced this pull request Aug 26, 2016
@laanwj laanwj Merge #8583: Show XTHIN in GUI
4c3e2cb Show XTHIN in GUI (R E Broadley)
c19f8a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment