ethereum / trin

An Ethereum portal client: a json-rpc server with nearly instant sync, and low CPU & storage usage
365 stars 113 forks source link

feat(beacon-network): add content validation #1320

Closed ogenev closed 3 months ago

ogenev commented 3 months ago

What was wrong?

We don't validate the beacon content type before storing and gossiping.

How was it fixed?

To-Do

pipermerriam commented 3 months ago

Reading through the validation code here, I wanted to ask what the status of our hive tests is for validating these objects. It seems like with as many validation checks as there are it would be easy for a client to miss or incorrectly implement the validation logic. If we don't have comprehensive tests vectors in place for both valid and invalid payloads, that seems like a place where effort should go. At minimum, getting issues written in the specs repo that detail the tests that need to be written.

KolbyML commented 3 months ago

Reading through the validation code here, I wanted to ask what the status of our hive tests is for validating these objects. It seems like with as many validation checks as there are it would be easy for a client to miss or incorrectly implement the validation logic. If we don't have comprehensive tests vectors in place for both valid and invalid payloads, that seems like a place where effort should go. At minimum, getting issues written in the specs repo that detail the tests that need to be written.

Currently there is no negatives, and we don't really test for positives either

ogenev commented 3 months ago

Reading through the validation code here, I wanted to ask what the status of our hive tests is for validating these objects. It seems like with as many validation checks as there are it would be easy for a client to miss or incorrectly implement the validation logic. If we don't have comprehensive tests vectors in place for both valid and invalid payloads, that seems like a place where effort should go. At minimum, getting issues written in the specs repo that detail the tests that need to be written.

Yeah, we will work on adding more tests for testing content validations for history and beacon. We are currently trying to fix the failing beacon hive tests for Trin.