Do not overwrite existing witness hashes for a given block in the DB
gsalgado opened this issue · 2 comments
Hm, just realized/noticed that we do not neatly handle what happens if we get different answers from different peers. We just overwrite the last witness-hashes at the same block. This would be especially bad if the second peer to send us the witness hashes gives us a list with one hash (to grief us). We also append the block to the witness history, so our history gets shorter than intended...
This might be a problem soonish, but I think it can be all handled in the witness db (storing and reading a union of all the hashes instead of overwriting the previous), so I don't think it needs to be fixed in this PR.
Originally posted by @carver in #2103 (comment)
I can take a hack at this
So we need to be sure to handle any race conditions, like two hash sets being saved at roughly the same time, and they both read out an empty set of hashes for a given blockhash. So might need a lock.
Current plan is to return the union of all hashes reported by all peers. It's probably worse to collect not enough data than to collect too much. Someday we might have to deal with griefing by sending way too many hashes, but for now I think empty witness griefing is a bigger concern.
Someday we might want something like a Counter() of the hashes returned by all peers, but for now I think a simple union of the set is the best next step.