Skip to content

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Dec 21, 2015

Replaces #6625 (contains commits from #6526).

…tion number

This is a no-op change. For now, everything passes MAX_BLOCK_SIZE / 60, so the
result matches what it would've before.

Tests use a number equal to the number of transactions where necessary,
to ensure that they're never rejected when blocksizesize isn't being tested.
@maaku
Copy link
Contributor

maaku commented Dec 21, 2015

What's the reasoning for nMaxTransactions?

@jtimon
Copy link
Contributor Author

jtimon commented Dec 21, 2015

More flexibility in the code for when MAX_BLOCK_SIZE is no longer a constant.

@maaku
Copy link
Contributor

maaku commented Dec 21, 2015

Perhaps my question wasn't clear -- why limit the number of transactions at all? We should probably do overflow checks (which would mean a real limit of 4 billion transactions), but I'm not sure why this would ever be a purposefully, further constrained parameter.

@jtimon
Copy link
Contributor Author

jtimon commented Dec 21, 2015

I guess another option would be to just replace MAX_BLOCK_SIZE / 60 with a static constant in merkleblock.cpp.
What you are saying is that changing that constant to a huge value wouldn't be a hardfork and it's therefore easier to just decouple merkleblock.cpp from MAX_BLOCK_SIZE, correct?

@maaku
Copy link
Contributor

maaku commented Dec 22, 2015

That check is really just a sanity check, and IMHO shouldn't be there as Merkle trees are used for other purposes. There is no reason to explicitly limit the number of transactions, and a generic merkle tree validator shouldn't have such a limitation built into it.

@jtimon
Copy link
Contributor Author

jtimon commented Dec 23, 2015

Ok, so you suggest to just remove the check then?

@jtimon jtimon closed this Jan 7, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants