Fix for #6960 and new unit tests for testing dependent batches
Fixes #6960
InsertHeaders now properly handles null when a dependent batch arrives.
The added unit test also showcases different header batches that is allowed and not. Among them are a bit of strange behavior, since we allow a batch to start with null, but not empty.
This is not allowed batch 1-3 header2, header3
This is allowed batch 1-3 null,header2, header3
Also this will fix several instances of ArrayPoolList not being disposed in HeaderSyncFeed.
What types of changes does your code introduce?
- [x] Bugfix (a non-breaking change that fixes an issue)
- [ ] New feature (a non-breaking change that adds functionality)
- [ ] Breaking change (a change that causes existing functionality not to work as expected)
- [ ] Optimization
- [ ] Refactoring
- [ ] Documentation update
- [ ] Build-related changes
- [ ] Other: Description
Testing
Requires testing
- [x] Yes
- [ ] No
If yes, did you write tests?
- [x] Yes
- [ ] No
why missing headers is not allowed?
EDIT: Ah the response should have same number of items as request - it is protocol rules. But maybe we can work around it?
I was pointing it out because it seems a little odd to allow empty or null in the end, but only null in the start. So if the rule is: The response has to be the same length as reques size, we should not allow empty in the end either.
why missing headers is not allowed? EDIT: Ah the response should have same number of items as request - it is protocol rules. But maybe we can work around it?
I was pointing it out because it seems a little odd to allow empty or null in the end, but only null in the start. So if the rule is: The response has to be the same length as reques size, we should not allow empty in the end either.
So I think the logic is that response is not allowed to skip anything, but is allowed to trim at the end. In theory there is nothing about it in spec: https://github.com/ethereum/devp2p/blob/master/caps/eth.md#blockheaders-0x04