ETCM-167: Scalanet Discovery part 3

[aakoshh]

Description

In preparation of using the Scalanet version of Ethereum Discovery v4, the PR implements the adapters for functionality that Scalanet currently doesn't do on its own, that is, the RLP encoding and the ECDSA cryptography.

To support defining RLP encodings for the case classes that map 1-to-1 with the RLP structure of messages, the PR implements an RLPImplicitDerivations object that uses Shapeless to generate the codecs automatically.

The build possibly needs #759 to be merged to pass.

Important Changes Introduced

No changes yet, only implements the required mappings from Mantis to Scalanet.
Changes will be coming in https://jira.iohk.io/browse/ETCM-168

Testing

Unit tests with test vectors.

[aakoshh]

This one randomly returned 32 or 33 bytes.

[KonradStaniec]

hmm maybe not for now as it more valuable to integrate it quickly with rest of the system, but maybe we could make Secp256k1SigAlg as a class and do this operation only once as multiplying points on curve can be quite inefficient, and sign will be called for each packet.

[aakoshh]

We can add caching to this class as well, which would require less changes to the overall arrangement of now keys are passed around. But if by multiplying points you mean specifically that the public key has to be derived from the private ever time, I agree, it's unnecessary, the only reason I'm doing it is because sign takes an AsymmetricCipherKeyPair, but it only ever uses keyPair.getPrivate. So if parsing the bytes itself isn't too expensive, then I can just add another sign overload that takes the private key only, rather than the key pair, to avoid needlessly constructing both. Just didn't want to mess around with crypto to start with.

[aakoshh]

Ah, that's not true, it uses the public key for the calculation of v.

[aakoshh]

Turned into a class and added caching.

[KonradStaniec]

I have mixed feeling about this. On one side its great way to reduce boilerplate, on the other hand i really like to have specific messages having their format explicitly created as then it is really easy to compare those formats with specification. (like we are doing with all of our .p2p.messages)
Maybe we could add some test cases showing how those messages looks like in their RLP encoded form ?

[aakoshh]

You mean that I should have classes that compare to hand-crafted RLPList structures? How would I know they are correct? I thought the test vectors I found in EIP8 are convincing enough that all these messages correspond to what they should be.

I agree that it's somewhat convenient to see the mapping with your eyes, but in itself it doesn't convince at all that they correspond to the spec, there could still be a field missing or in the wrong order. It also looks like it could be easy to forget to include clause to capture the "rest" of the fields, like RLPList(a, b, _*) vs RLPList(a, b), where the second version my work today but fail later if a new version adds more fields, or as EIP8 mandates that we have to accept random garbage at the end. This commit had a version of hand-crafted codec, but with the trailing optional field it was a bit tricky (probably also wrong, encoding the optional field as an RLPList(Long) instead of Long).

I was hoping that the ethereum tests would have examples of everything but it doesn't seem to be the case.

[aakoshh]

Another way I thought could be a bit less repetitive than what's in .p2p.messages is to use the tuple codecs but I ended up not using them after all. With them the mapping would be something like this:

implicit val nodeAddressCodec = 
  implicitly[RLPCodec[(InetAddress, Int, Int)]].xmap(
   (Node.Address.apply _).tupled,
    Node.Address.unapply(_).get
  )

However I didn't try what kind of error message you'd get from that, while deriveLabelledGenericRLPCodec would tell you exactly which field failed to be decoded and why (it's better in that sense than the manual method relying on implicit conversions, any of which can fail at the same time).

[aakoshh]

Added examples for what the encoded RLP looks like.

[KonradStaniec]

sure i totally agree that test vectors are more reassuring than just looking at payloads (as this this tells nothing are those payloads transformed into actual bytes), but imo it is good to have the representation before your eyes in some cases. I think that simple test cases are good middle ground here.

[KonradStaniec]

LGTM!