Friday 31 March 2017

Anti-FUD: The BIP148 enforcing client - a walkthrough.

There seems to be a lot of FUD going around surrounding http://bit.ly/2oER6bR <--that little guy. I'm a programmer, so let me walk you through what, exactly, is changed, and how you can verify what changes for yourself.

So, to get started, click on the 'Compare' button just below the green button that says 'clone or download'. link

This shows you every single change that has been merged between bitcoin core, in the 0.14 branch (the branch that was used to create the 0.14 Core client many of us use) and this repository's version of the 0.14 client, which requires any blocks after August 1, 2017 to support Segwit.

So, let's go through the page, top to bottom, and explain what it is telling you.

19 commits 4 files changed 3 commit comments 3 contributors 

That tells you that 19 times someone has changed something in the code base, in total, 4 files were changed by those 19 commits, 3 commit comments were made (think of these as replies to a thread on reddit), and 3 people total have made contributions to the code differences represented below.

Below that is a list of what commits were made on what day. You can click on the second column (BIP148 / Update client name to Satoshi BIP148 / etc) to see what changes were made in that version (compared to the version before it) specifically.

Scroll down until you hit

Showing with 19 additions and 5 deletions. 

This is where the 'fun' (programming) begins.

src/clientversion.cpp

-std::string FormatSubVersion(const std::string& name, int nClientVersion, const std::vector<std::string>& comments) +std::string FormatSubVersion(const std::string& name, int nClientVersion, const std::vector<std::string>& comments, const bool fBaseNameOnly) 

Red lines, which always start with a minus sign, means that line was removed from the file. Green lines, which always start with a + sign, mean that line was added. "But the line wasn't removed, just some stuff was added to the end!" Correct! This is a 'diff-ism'. Diff being the name of the program used to show differences between a file. Diff doesn't highlight just the part of the line that changed, it highlights the entire line, and leaves it to you to spot the changes in the line.

From the above, we can see a parameter was added to the end of the line. "But what does the line do!" Well, what you're looking at is a function declaration. What is a function? Well, imagine you wanted to build a robot to make sandwiches for you. You could make the sandwich yourself, but it's easier if an automated system does it for you. The function is like the robot; you put a specific set of tasks into the robot's programming, give it a specific set of inputs (bread, knife, meat/cheese/spreads/etc) and it returns the resultant sandwich. The way to read the declaration is this:

std::string FormatSubVersion(const std::string& name, int nClientVersion, const std::vector<std::string>& comments, const bool fBaseNameOnly) 
  1. std::string The first argument is the return type of the function. In this case, a C++ string.
  2. FormatSubVersion This is the name of the function
  3. (const std::string& name, the first parameter of the function, since it is unchanged from Core, and unmodified by other changes in the file, I will not bother explaining what it does.
  4. int nClientVersion, Second parameter to the function. Same thing, original, unmodified, skipping.
  5. const std::vector<std::string>& comments, Parameter 3, unchanged, skipping.
  6. , const bool fBaseNameOnly) Parameter 4, 'const bool' means two things: 1) we cannot change the value of this variable in the code. 2) it's a 'bool' type, which is short for boolean. It an either be true or false, those are the only values it can ever have. What does it do? Let's keep reading.
std::ostringstream ss; 

That's important for later, make note of it.

if (!fBaseNameOnly) ss << "UASF-Segwit:0.2(BIP148)/"; 

The above is the change uses the newly minted parameter 4 to add a bit of text into the output stream. Specifically, the string "UASF-Segwit:0.2(BIP148)/" is tacked on to whatever is ahead of it in the output stream. The net result of this change is that clients using this code will report their client version as '/Santoshi:0.14.0/UASF-Segwit:0.2(BIP148)/' instead of the standard value of '/Santoshi:0.14.0/'.

File complete! Next file.

src/clientversion.h

Within C or C++ programming, you have the concept of 'code files' (ending in .c or .cpp) and 'header files' (ending in .h). Strictly speaking, any code can be in either file and the compiler will figure it out (assuming you give it enough information to do so). However, programming conventions exist. Since I assume the readers of this post are (largely) not programmers, I won't bore you. It's a convention used for sanity only, and it is a convention followed by the bitcoin source code. In general, program code that 'does stuff' goes in .c and .cpp files, and the code needed to tell the compiler (compiler = the thing that converts these text files into a program) where to 'find stuff' goes into .h files.

-std::string FormatSubVersion(const std::string& name, int nClientVersion, const std::vector<std::string>& comments); +std::string FormatSubVersion(const std::string& name, int nClientVersion, const std::vector<std::string>& comments, bool fBaseNameOnly = false); 

Well, because this is the exact same function call we just talked about in the previous section, I'll skip going through the parameters one by one, and instead focus only on the change: , bool fBaseNameOnly = false).

"WAIT! It has 'const' before bool in the .cpp file! That's bad right!?" No. The compiler will see const in the .cpp file and mandate the variable be const.

"WAIT! Here it says '= false' and in the .cpp file it doesn't!" Again, not a problem. Remember how I said some code goes in .c/.cpp files, and some in .h files? Well, this is a case where which file contains what code actually does matter. Basically, you can't set a default value for a parameter inside a .c/.cpp file. You can only do that in a .h file. So...that's 100% correct. Here is the souce code for a quick little program to see this behavior:

--test.cpp--

#include "test.h" #include <stdlib.h> #include <stdio.h> int main() { function(); } int function(const bool tmp) { tmp = !tmp; } 

---test.h---

int function(bool test = false); 

--If you tried to compile this, you'd get--

g++ test.cpp test.cpp: In function ‘int function(bool)’: test.cpp:12:6: error: assignment of read-only parameter ‘tmp’ tmp = !tmp; 

In this case, 'read only' means 'was declared const'.

Remember how a 4th parameter was added in the code above? Well, you have to tell the compiler to expect that parameter, which you do here, in the header file. That line of code tells the compiler to expect the 4th parameter. It also sets the default value of the parameter, should the caller not specify it, to be false.

Thus, you can call this function two ways:

  1. FormatSubVersion("Test", 99900, std::vector<std::string>())
  2. FormatSubVersion("Test", 99900, std::vector<std::string>(), true)

Using method 1 would result in a User Agent string of '/Test:99900/UASF-Segwit:0.2(BIP148)/', because the program uses the default value of 'false' and so it sticks in the bit about BIP148 support. Using method 2 would result in '/Test:99900/' "Wait, wait, how did you figure that out?" Look here, scroll to the bottom (line 88) and that is the FormatSubVersion function we went over above. All you do is built the string in steps as you read the code:

  1. Line 90: ""
  2. Line 91: "/"
  3. Line 92: "/Test:99900" {the 'Test' comes from the 'name' parameter, parameter 1. The : is statically coded (<< ":" <<) and the 99900 comes from nClientVersion, parameter 2}
  4. Line 93: From the function call, we see that parameter 3 is initialized 'std::vector<std::string>()', this is an empty vector. If the vector had anything in it, it would look like this: std::vector<std::string>('a')
  5. (because the if statement in line 93 fails, we go to: ) Line 101: "/Test:99900/"
  6. Line 102: (are we doing a version with or without the 4th parameter set to true?)
  7. Line 103: (if parameter 4 is false, line becomes "/Test:99900/UASF-Segwit:0.2(BIP148)/"
  8. Line 104: Convert the 'ss' variable to a standard C++ string and return the that string to whatever asked this function to be run.

SO, in total, this function literally just creates a string. Much like the robot-sandwich example, you give the function a client name, version, and list of comments and it builds you a string containing those things.

src/test/util_tests.cpp

This file is part of the automated testing for bitcoind/bitcoin-qt. When you compile the software, you'd typically run 'make check' before installing the software, to ensure that your changes didn't break anything and that your compile didn't go wrong. With the effort I've put into explaining the change to FormatSubVersion in the past two section, I believe you can now see that the only change made to this test is to ensure that the newly added code performs as expected.

That said, there is a 'defect' in this code. He should not have removed the 3 existing tests. He should have added 3 new tests. That way he'd have both 'positive' and 'negative' test case coverage. That said, it isn't something to fret about.

src/validation.cpp

All right, finally, the big file where all the cool shit happens!

+ // BIP148 mandatory segwit signalling. + if (pindex->GetMedianTimePast() >= 1501545600 && // Tue 1 Aug 2017 00:00:00 UTC + pindex->GetMedianTimePast() <= 1510704000 && // Wed 15 Nov 2017 00:00:00 UTC + !IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus())) + { + // versionbits topbit and segwit flag must be set. + if ((pindex->nVersion & VERSIONBITS_TOP_MASK) != VERSIONBITS_TOP_BITS || + (pindex->nVersion & VersionBitsMask(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) == 0) { + return state.DoS(0, error("ConnectBlock(): relayed block must signal for segwit, please upgrade"), REJECT_INVALID, "bad-no-segwit"); + } + } + 

The entire section is newly added. Anything it does will be 'in addition to' whatever is already done. Let's go through the change line by line:

  • Line 1854: Add a comment (comments can begin many ways, // is one of those ways. Anything after a // on a line will be part of the comment.
  • Line 1855: GetMedianTimePast is defined in src/chain.h (line 295-307) and returns the average timestamp of the last 11 (or fewer) blocks. Basically, because there are more than 11 blocks in the blockchain, and assuming you aren't looking at the first 10 blocks, this code will always give you the median time at which the last 11 blocks occurred. So..., let's say the last 11 blocks occurred at: 100,120,140,190,210,230,240,250,270,275,290 this function would return: (11 - 0) / 2 = 5, so it would return the 6th (computers count from 0) element of that list, so 100 is the 0th element, 120 is the 1st, so it would return 230 as the 'Mean' time.

"Ok, but what about 1501545600? How do we know that?" It's an epoch timestamp. Google 'epoch converter', copy-paste that number in, convert to UTC, and you'll see it is correct for what the comment says it is.

The '&&' at the end of the line means 'and'. So in this case, 'if the mean age of the past few blocks is greater than or equal to <date1> and ...'

  • Line 1856: 'the mean age of the past few blocks is less than or equal to <date 2> AND...'
  • Line 1857: Use's Core's own function to check whether the previous block (to the current one we are validating) support Segwit. So, at this point, the logic says: 'if the mean age of the past few blocks is greater than or equal to <date1> AND the mean age of the past few blocks is less than or equal to <date 2> AND the previous block did NOT support segregated witness, then do line 1859's actions (the bit inside the {}), otherwise, go to line 1865.
  • Line 1860: This uses a bitwise-AND mask to determine what features the block supports. The mask is defined in src/versionbits.h Version bits are a feature of BIP9 which is easily explained at a high level at that link. In essence, this if statement says "If this block supports BIP9...". This is hard for the non-programmer to understand by reading, so I'll explain it this way: If you use the bitwise-and mask on a block with the format of a non-BIP9 block, then the result of the bitwise-and is the value of VERSIONBITS_TOP_BITS. That is why it says !=, which means 'not equal to'.

You can see proof of this claim in the tests written in src/test/versionbits_tests.cpp lines 277-281. line 277 creates an 'old format' block, then (line 279) checks that the ComputeBlockVersion function works, then verifies that the bitwise-and function returns TOP_BITS, as expected.

  • Line 1861: Parses the version mask to see if the mask signals support for SEGWIT. As of this line, the inner if statement reads: 'If this block does not support BIP9 messaging, or this block does not signal support for SEGWIT, then go to line 1862, otherwise, go to line 1863.
  • Line 1862: Reject the block, stating that the block must support SEGWIT.

If you are concerned that more might be needed to reject a block, simply view src/validation.cpp on line 1892 and see that standard bitcoin Core code rejects blocks in the same way as the SEGWIT patch does.

"So wait, what is the total requirement to reject a block again?"

  1. If the mean age of the past few blocks is greater than or equal to <date1> AND the mean age of the past few blocks is less than or equal to <date 2> AND the previous block did not show that Segwit was in 'active' state:
  2. If all of the conditions in step 1 are met AND the block either does not support BIP9 messaging, or does not signal support for SEGWIT
  3. Then it will be rejected.

"So wait, what happens after the first segregated witness block pops across the network? Hasn't that already happened?" No. Blocks that support segwit have come across the network, but in order for IsWitnessEnabled to return 'true', the SEGWIT state would need to switch to 'active' (see BIP9 spec), which is the final state of any proposal, and the point at which the setting is considered an accepted part of the blockchain.

Conclusions

So, you see, no muss, no fuss. The day-1 bug where the logic was backwards has been fixed. There is nothing to fear. Feel free to ask questions and I'll explain them over the next few hours/days as I am able. I'll try to talk to your level if I can. I like teaching in general and abhor ignorance in all its forms. Understand: ignorance strictly means 'not knowing', rather than the typical 'negative' connotation it gets in English speaking society. I would like everyone to realize just how simple this UASF patch is and that the FUD surrounding it not being 'verified' is absolutely a bad joke.

edit: Logic fix thanks to Phil. Like shaolinfry, I had my negated logic backwards. Oops.



Submitted March 31, 2017 at 07:05AM by Kingdud http://bit.ly/2nR87Cv

No comments :

Post a Comment