Allow for messages to be up to 32MB
ClosedPublic
Actions

Authored by deadalnix on Jun 12 2017, 21:11.

Details

Reviewers

freetrader
sickpig
kyuupichan

Group Reviewers

Restricted Project

Commits

rSTAGINGd757be14a738: Allow for messages to be up to 32MB
rABCd757be14a738: Allow for messages to be up to 32MB

Summary

This ensure we can support bigger blocks, at least long enough so we'll see it comming.

I added test for blocks up to 16MB in the integration test to make sure this works. Without the message size limit, this doesn't work.

Test Plan

make check
../qa/pull-tester/rpc-tests.py abc-p2p-fullblocktest-simple.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

deadalnix created this revision.Jun 12 2017, 21:11

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 12 2017, 21:11

nits

freetrader mentioned this in D183: Implement sigops checks in abc-p2p-fullblocktest.py.Jun 13 2017, 01:06

I would be hesitant to set the message limit to 32 MB if we are only testing up to 16MB block size. If we were able to successfully test up to 32M blocks I would be fine with setting the MAX_PROTOCOL_MESSAGE_LENGTH to 32MB. My view is we should increase it as we eliminate the obstacles and test it successfully at higher sizes.

What is the downside of keeping the MAX_PROTOCOL_MESSAGE_LENGTH tied to exactly the block size limit?

Also, do you already have an idea why it fails when trying 17MB ?
Setting MAX_PROTOCOL_MESSAGE_LENGTH to 16MB and testing up to 16MB seems to work alright.

The > 16MB might be something with the framework as I don't see an Oversize error at that size.

When setting to 32MB I get the Oversize error triggered in the client (and the test hangs due to this).

freetrader added inline comments.Jun 13 2017, 01:18

src/net.h
61 ↗	(On Diff #412)	Consider setting this to 16MB which is max tested blocksize, and increasing it to 32MB only once we succeed with blocks up to that limit.

deadalnix added inline comments.Jun 13 2017, 09:46

src/net.h
61 ↗	(On Diff #412)	We don't want to be at capacity there.

What is the downside of keeping the MAX_PROTOCOL_MESSAGE_LENGTH tied to exactly the block size limit?

That limit is not available easily at that place in the code and making it available would be too big. The goal here is to have a nice margin that'll last long enough so we can actually do this properly. 32MB is fine and in fact was the limit for a long time.

Also, do you already have an idea why it fails when trying 17MB ?

Does it ? I set excessive block size on the node to be 16MB, but if put higher, I'd expect things to fail slightly before 32MB .

It worked for me up to 16MB but not 17.

I would be interested whether others can reproduce that.

So I think there is a chance that you actually do not have the nice margin you are hoping for.

freetrader added inline comments.Jun 13 2017, 10:23

src/net.h
61 ↗	(On Diff #412)	For me it is a question of only allowing the software to run in a well-tested envelope where you can say it was verified to work (according to our current best efforts). Outside of that envelope, one enters the realm of wishful thinking.