Xtreme Thinblocks ready for code review PR#10

sickpig

Active Member
Aug 28, 2015
926
2,541
@Peter Tschipper

I'm going to test this feature, just want to be sure if xthinblock_12.0 is the correct branch to build the executable from?


edit: in the meantime I've tried to compile the xthin_12.0 branch and I got those warnings:

Code:
main.cpp: In function ‘bool ProcessMessage(CNode*, std::string, CDataStream&, int64_t)’:
main.cpp:4460:18: warning: the address of ‘bool IsThinBlocksEnabled()’ will always evaluate as ‘true’ [-Waddress]
  if (!IsThinBlocksEnabled)
  ^
main.cpp:4536:32: warning: the address of ‘bool IsThinBlocksEnabled()’ will always evaluate as ‘true’ [-Waddress]
  if (IsThinBlocksEnabled)
  ^
main.cpp:4977:25: warning: the address of ‘bool IsThinBlocksEnabled()’ will always evaluate as ‘true’ [-Waddress]
  } else if (!IsThinBlocksEnabled) {

Code:
  CXX  libbitcoin_server_a-net.o
At global scope:
cc1plus: warning: unrecognized command line option "-Wno-self-assign" [enabled by default]
  CXX  libbitcoin_server_a-noui.o
Code:
db/memtable.cc: In member function ‘void leveldb::MemTable::Add(leveldb::SequenceNumber, leveldb::ValueType, const leveldb::Slice&, const leveldb::Slice&)’:
db/memtable.cc:104:34: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  assert((p + val_size) - buf == encoded_len);
  ^

Code:
util/bloom.cc: In member function ‘virtual void leveldb::{anonymous}::BloomFilterPolicy::CreateFilter(const leveldb::Slice*, int, std::string*) const’:
util/bloom.cc:50:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  for (size_t i = 0; i < n; i++) {
  ^

Code:
util/logging.cc: In function ‘bool leveldb::ConsumeDecimalNumber(leveldb::Slice*, uint64_t*)’:
util/logging.cc:58:53: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
  ^
 
Last edited:

theZerg

Moderator
Staff member
Aug 28, 2015
1,012
2,327
I fixed these... give me a few hrs to checkin and you can use BUIP010 branch in BU repo
 

theZerg

Moderator
Staff member
Aug 28, 2015
1,012
2,327
Ok, it is ready. And you can try connecting to 66.222.25.72...
 

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@theZerg

There's a bug in net.cpp which I pushed up late yesterday, i guess it didn't get included. This is what it should be.

void ThreadOpenConnections()
{
// Connect to specific addresses
if ((mapArgs.count("-connect") && mapMultiArgs["-connect"].size() > 0) ||
(mapArgs.count("-connect-thinblock") && mapMultiArgs["-connect-thinblock"].size() > 0)) // BUIP010 Xtreme Thinblocks
[doublepost=1455550960][/doublepost]@theZerg BTW i'm connected to you so you should see some xthin requests in your log now.
 
Last edited:

sickpig

Active Member
Aug 28, 2015
926
2,541
@theZerg @Peter Tschipper

I can confirm that compiling BUIP010 branch produces a warning related to the code in net.cpp Peter mentioned in the prev post:

net.cpp: In function ‘void ThreadOpenConnections()’:
net.cpp:1472:35: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
if (mapArgs.count("-connect") && mapMultiArgs["-connect"].size() > 0 ||
 

theZerg

Moderator
Staff member
Aug 28, 2015
1,012
2,327
AFAICT I am not receiving any thin blocks yet, although some of my nodes are still synchronizing...

I would like to optimize this so that thin block capable clients prefer getting blocks from each other.

It looks like the code responds via a thin block to clients that request thin blocks. So we essentially have an efficiency race condition here where if an INV is received from a thin-block node then then our node requests a thin block, otherwise it requests a fat block.

So we need to send INVs out as fast as possible to thin block clients so they choose us to request from (an additional option is to delay requesting a block from an incoming fat-block INV). Maybe both options makes the most sense but I haven't looked at the difficulty of implementing the fat-block delay...

I'm looking at net.cpp:ThreadMessageHandler(), where it does:
foreach (node) { receive_handler(); }
foreach (node) { send_handler(); }

What if we did:
foreach (thin-capable node) { receive_handler(); }
foreach (thin-capable node) { send_handler(); }
foreach (node) { receive_handler(); }
foreach (node) { send_handler(); }

I think that that would have the effect I am looking for. What do you think?
 

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@theZerg Some nodes may not receive thinblocks as there has to be some entry way into the thinblock node network. So I"m not sure you can favor thinblock nodes without the risk of splitting the network, or at least we would have to make very sure that doesn't happen.

Can I ask what how many thinblock nodes your connected to? There is a small problem such that if a node connects to two nodes with the same IP but different ports but doesn't have a third connect-thinblock then it would appear as a crossconnected node and download a regular block. This would be exceedingly rare and I believe that's what you may have done since we don't have that many nodes up right now. I see you have a connection in to both my nodes which are on different ports but same ip?

You said:
"It looks like the code responds via a thin block to clients that request thin blocks. So we essentially have an efficiency race condition here where if an INV is received from a thin-block node then then our node requests a thin block, otherwise it requests a fat block"

Actually there is no efficiency race...if you use "connect-thinblock" it will *only* download from a thinblock node unless there is some kind of cross-connect going on. If it can't immediately download a thinblock it will spin around until it gets an INV from a thinblock node.

If you haven't already done so you could also add 104.197.99.28 as a connect-thinblock node. That should clear up the issue of the false cross connect.
 

sickpig

Active Member
Aug 28, 2015
926
2,541
@Peter Tschipper

I've cherry picked the fix you mentioned above and it fixed the warning.

This is how I set up the test:

- bitcoind compiled disabling wallet function
- copy an almost up-to-date datadir from a working 0.11.2 client
- use above copied dir as the data dir for the new client
- execute bitcoind with the following options:
Code:
 -connect-thinblock=66.222.25.72 -daemon -datadir=/path/to/copied/0.11.2
the node is currently catching up (height=398609)

a few questions:

1) how can I specify more than one thinblock peer, passing comma separetd ip address to the connections thin block options?

2) what I have to look for in debug.log?
 

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@sickpig

to specify more than one peer, just add more entries one on top of the other like:

connect-thinblock=1.2.3.4
connect-thinblock=1.2.3.5
connect-thinblock=1.2.3.6


and to see all the log data use debug=thin It will be pretty obvious in the debug log, you'll see references to xthinblock, bloom filter, timing data for downloading a block etc...

One thing though, as mentioned above if you're going to connect to both my nodes at 96.54.102.88 then make sure you select a third node as well. Connecting to two nodes only with the same ip but different ports causes a cross connect to be flagged and you'll end up downloading a full block.
 

sickpig

Active Member
Aug 28, 2015
926
2,541
@Peter Tschipper

thanks, I'm going to change my conf accordingly.

On a related note do we happen to have a more real time way to communicate? like IRC? This ping pong forum based exchange does not fit IMHO.
 

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@sickpig IRC? I don't think we do have that or do we? I'm never one to enjoy IRC except for things like this where we have an upcoming release and we need to move more quickly.
 

sickpig

Active Member
Aug 28, 2015
926
2,541
@Peter Tschipper dunno if we already have an IRC channel but creating one on freenode is quite easy and free. Tomorrow I'll have a look.

that said I'm actually running out of available testing time for today.

I'm going to let the node running for a few hours and tomorrow I'll report any significant results.

Update: node is synced now, still I can't see any trace of thin debugging output on my debug.log. This is my bitcoin.conf:

Code:
debug=thin
connect-thinblock=66.222.25.72
connect-thinblock=104.197.99.28
connect-thinblock=96.54.102.88
Update 2:

scratch that, as soon as new block was announced thin debug string started to appear

Code:
$> grep -Ei '(thin|bloom)' ./debug.log
2016-02-15 22:47:18 Starting creation of bloom filter
2016-02-15 22:47:18 Bloom multiplier: 1.403600 FPrate: 0.001541 Num elements in bloom filter: 2706 num mempool entries: 1928
2016-02-15 22:47:18 Sending bloom filter: 4570 bytes peer=4
2016-02-15 22:47:18 Requesting Thinblock 0000000000000000037b623061bb5c6fa329b8b12ed90bbe3a1c66a776bef929 peer=4
2016-02-15 22:47:25 received thinblock 0000000000000000037b623061bb5c6fa329b8b12ed90bbe3a1c66a776bef929 peer=4 (330656 bytes)
2016-02-15 22:47:25 Reassembled thin block for 0000000000000000037b623061bb5c6fa329b8b12ed90bbe3a1c66a776bef929 (949204 bytes)
2016-02-15 22:47:26 Removing Thinblock in flight 0000000000000000037b623061bb5c6fa329b8b12ed90bbe3a1c66a776bef929  peer=4
2016-02-15 22:52:59 Starting creation of bloom filter
2016-02-15 22:52:59 Bloom multiplier: 1.491000 FPrate: 0.001054 Num elements in bloom filter: 268 num mempool entries: 180
2016-02-15 22:52:59 Sending bloom filter: 490 bytes peer=3
2016-02-15 22:52:59 Requesting Thinblock 00000000000000000644f86d77daba34a6dd9bbe3ae7658da2bea9c619bd17f0 peer=3
2016-02-15 22:53:08 received thinblock 00000000000000000644f86d77daba34a6dd9bbe3ae7658da2bea9c619bd17f0 peer=3 (457644 bytes)
2016-02-15 22:53:08 Reassembled thin block for 00000000000000000644f86d77daba34a6dd9bbe3ae7658da2bea9c619bd17f0 (514393 bytes)
2016-02-15 22:53:08 Removing Thinblock in flight 00000000000000000644f86d77daba34a6dd9bbe3ae7658da2bea9c619bd17f0  peer=3
 
Last edited:

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@theZerg I'm not sure I was very clear in my explanation but basically how connect-thinblock works right now is that IF you are using "connect-thinblock" then you will always get a thinblock.

UNLESS there is a cross connection. A cross connection meaning, "You're connected to me using "connect-thinblock" and I'm connected to you using "connect-thinblock" AND there are no other thinblock nodes that you are connected to using "connect-thinblock". It's not really that easy to do except when we have only a few nodes running.

Also, getting a cross connect doesn't mean you will get a full block, you still may get a thinblock but if none are available it will then download the full block without waiting any longer.
 

theZerg

Moderator
Staff member
Aug 28, 2015
1,012
2,327
I wasn't using connect-thinblock... I was just expecting that 2 thin block nodes that connected would use thin blocks. Is that the case?
 

Peter Tschipper

Active Member
Jan 8, 2016
254
357
@theZerg If most nodes were using thinblocks then yes that would be the case...but because there are not many we can use "connect-thinblock" to force the downloading of thinblocks until the time comes that it's more widely supported by other nodes.

But you were right in that we don't need to use connect-thinblock but it's just not likely that you'll get a thinblock right now with so few nodes out there.
 
Last edited: