Page MenuHomePhabricator

Accept network messages for 32MB blocks and larger.

Authored by deadalnix on Feb 25 2018, 13:08.



This patch ensure that all message are capped to 1MB, except for message that can possibly containt the block content. In this case, the size of the accepted message is scaled with the accepted block size.

Test Plan

Upated intergation tests to check it is now possible to accept blocks of 32MB.

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

Herald added a reviewer: Restricted Project. · View Herald Transcript
kyuupichan added inline comments.
65 ↗(On Diff #2978)

So is it 2MB or 1MB?

jasonbcox requested changes to this revision.Feb 25 2018, 20:42
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
715 ↗(On Diff #2981)

What's the purpose of this? It seems like for the foreseeable future, max block size will be quite a bit larger than MAX_PROTOCOL_MESSAGE_LENGTH, rendering this if-statement useless.

If you feel that there's some worth to scaling the accepted message size, perhaps MAX_PROTOCOL_MESSAGE_LENGTH should be scaled according to max block size instead?

This revision now requires changes to proceed.Feb 25 2018, 20:42
deadalnix added inline comments.
715 ↗(On Diff #2981)

Why would you accept absurdly large protocol message for anything that's not the content of a block ? It just opens you to DoS with zero benefits.

715 ↗(On Diff #2981)

You wouldn't. I'm saying this if-statement will not be reached if msg.hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH, where MAX_PROTOCOL_MESSAGE_LENGTH is always (usually?) < 2 * config.GetMaxBlockSize().

715 ↗(On Diff #2981)

It also checks for NetMsgType::IsBlockLike . It'll fail only if this isn't a message that can contain a block content.

This revision is now accepted and ready to land.Feb 26 2018, 16:43
This revision was automatically updated to reflect the committed changes.