Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the max-block-size that is used in new networks by default #419

Merged
merged 2 commits into from
Dec 11, 2017

Conversation

xeroc
Copy link
Member

@xeroc xeroc commented Oct 13, 2017

... unless the genesis block is modified manually, or the committee
changes that parameter on the chain.

This fixes the issue that blocks might otherwise become bigger than the
MAX_MESSAGE_SIZE of the p2p code. This far, I am unaware that the p2p
does segmentation of messages.

... unless the genesis block is modified manually, or the committee
changes that parameter on the chain.

This fixes the issue that blocks might otherwise become bigger than the
MAX_MESSAGE_SIZE of the p2p code. This far, I am unaware that the p2p
does segmentation of messages.
@abitmore
Copy link
Member

Do a replay to make sure it didn't break anything.
If it's really an issue, the committee should change the parameter to a lower value (asap?), and never set it to a higher one before this issue is totally fixed (e.g. p2p message segmentation).

@dnotestein
Copy link

dnotestein commented Oct 13, 2017

I agree that the blocks shouldn't be allowed to be larger than MAX_MESSAGE_SIZE. Messages are not segmented by the p2p code. It's probably a good idea to test that there's no overhead that makes the final message larger than the nominal block size (for example, by creating a block of max size and trying to propagate it).

As a related issue, I don't know why 2MB was chosen as the max message size, but this could certainly be increased some.

@pmconrad
Copy link
Contributor

Setting max block size to the same as max message size won't help, because of the message overhead. You need to stay below that, for example use 2000000 to be on the safe side.

@xeroc
Copy link
Member Author

xeroc commented Oct 16, 2017

Someone just exploited this on the testnet

@xeroc
Copy link
Member Author

xeroc commented Oct 16, 2017

For reference, this is how you can truncate the blockchain locally:

ls -l .../blockchain/index
# look at size in bytes
# new= subtract 32 * number of blocks
truncate -s new_size blockchain/index

truncating the last 32 bytes effectively pops the last block from your block db

replay is needed

Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @pmconrad mentioned, best change it to an even smaller value.

@abitmore abitmore added this to the Next Non-Consensus-Changing Release - 201712 milestone Nov 28, 2017
@abitmore abitmore merged commit b860946 into develop Dec 11, 2017
@abitmore abitmore deleted the reduce-genesis-max-block-size branch December 12, 2017 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants