Change rlp to alloy-rlp

[armaganyildirak]

Description

Try to change rlp to alloy-rlp.

Notes & open questions

Some of the tests are not working. Probably I need help for fixing them.
I import my latest pr sigp/enr#46 as enr because I need the use alloy-rlp for both repo.

enr = { git = "https://github.com/armaganyildirak/enr", rev = "8c445fb31b8d5190ea32429fc360750e898dfe3f", features = ["k256", "ed25519"] }

[armaganyildirak]

When I tried below inputs for decoding and encoding
1-

        let data = [6, 193, 0, 63];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([0]), body: Talk { response: [63] } })
  data: [6, 193, 0, 63]
 encoded: `[6, 194, 0, 63]

2-

        let data = [6, 193, 128, 75];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([]), body: Talk { response: [75] } })
  data: [6, 193, 128, 75]
 encoded: [6, 194, 128, 75]

3-

        let data = [6, 193, 128, 128];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([]), body: Talk { response: [] } })  
  data: [6, 193, 128, 128]
 encoded: [6, 194, 128, 128]

[divagant-martian]

Giving you here some ideas of what going on so that maybe you can figure out the bugs:

When I tried below inputs for decoding and encoding 1-

        let data = [6, 193, 0, 63];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([0]), body: Talk { response: [63] } })
  data: [6, 193, 0, 63]
 encoded: `[6, 194, 0, 63]

6 is the kind identifier, and [193, 0, 63] is the rlp data. This means: a small list with 1 byte of payload (192 + 1 = 193). So in theory this should read only the 0 and consider the 63 extra data that is rejected. So basically this is a case of accepting valid rlp with extra data. Given the extra data (and the fact that this does not decoed to anything we care about) this should be rejected

2-

        let data = [6, 193, 128, 75];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([]), body: Talk { response: [75] } })
  data: [6, 193, 128, 75]
 encoded: [6, 194, 128, 75]

Same as before, only the 194 one is correct. in these responses the whole rlp must be a list of two elements (elements - do not confuse with bytes) it seems to be you are reading [193 128] as valid rlp data (which it is), followed by 75 (also valid rlp) and them putting them together. But this does not commute, the whole thing needs to be contained in the rlp list, not two pieces of rlp data concatenated. Try to find the culprit around that logic

3-

        let data = [6, 193, 128, 128];
        let msg = Message::decode(&data).unwrap();
        let encoded = msg.encode();
        assert_eq!(data.to_vec(), encoded);

the result is:

Response(Response { id: RequestId([]), body: Talk { response: [] } })  
  data: [6, 193, 128, 128]
 encoded: [6, 194, 128, 128]

Exactly as before

[AgeManning]

This initially looks good to me. @ackintosh - If you have a spare sec, could you run this against some real nodes and see if it functions as we expect.

[AgeManning]

Also @armaganyildirak - Can you target a released version of the ENR crate so we can merge this in.

[armaganyildirak]

@AgeManning We merged my enr alloy-rlp PR after the v0.10.0 so this PR is not working with enr v0.10.0. But this PR is working with latest enr commits.

[AgeManning]

ahh ok, So we need a new ENR release. Cool. I'll get that sorted, thanks

[armaganyildirak]

Yes thank you. I will change the enr version with new one after we need a new discv5 release because this two PRs are working together.

[ackintosh]

I'm sorry for not noticing this PR earlier. Currently, I'm conducting tests for this PR using Testground (discv5-testground), and everything looks good so far. I'm also working on creating tests for TALKREQ/TALKRESP. I'll let you know once the tests are completed.

[ackintosh]

The tests have been successfully completed. 🙂

[armaganyildirak]

@AgeManning. I changed enr version to 0.10.1. This PR is ready to merge and we need a new discv5 release.

[AgeManning]

Awesome. Thanks @armaganyildirak - I've yanked 10.1, can we target 11.0 now. Then lets merge this guy down.

[armaganyildirak]

I changed enr version 10.1 to 11.0.