paritytech / json-rpc-interface-spec

29 stars 3 forks source link

should `prunedBlockHashes` be allowed to have repeated block hashes? #142

Closed josepot closed 3 weeks ago

josepot commented 6 months ago

The current spec reads:

prunedBlockHashes contains, in no particular order, a list of blocks that are not descendants of the latest finalized block. These blocks will never be finalized and can be discarded.

I always assumed that prunedBlockHashes would contain an unsorted list of unique block-hashes. However, the spec doesn't mention the "uniqueness" attribute of the list. Should we amend the spec or is there a reason why it makes sense to send repeated items?

tomaka commented 6 months ago

I don't see in which circumstances it would make sense to have multiple times the same hash here. I also don't see how a duplicate entry could be anything else but a very serious bug in the server.

lexnv commented 6 months ago

Indeed, reporting duplicate hashes is an issue on the server side. We could mention that the reported pruned blocks are unique to remove ambiguity.

tomaka commented 6 months ago

I think that mentioning this is more confusing than not. If you say it's unique, it implies that there's a world where it might have been non-unique. Having a non-unique list of hashes is so fundamentally wrong that there shouldn't even be a possibility that they're non-unique.

lexnv commented 3 weeks ago

I think we can close this issue since the fix was merged in substrate a while ago

@josepot let me know if there's something else we can do to the spec to clarify this, feel free to reopen this any time 🙏