Fix expected data type when decoding RLP in PeersConnection
Fill free to look at the discussion that led to this issue. When decoding RLP data, we should be expecting a particular type of data, so the type should be matched as well, instead of using _vsn.
We do handle types, I don't think we should care about versions really, just have the latest version for the different messages
When you use TYPE.rlp_decode or Serialization.rlp_decode_only so the type is checked.
I think what currently is missing is some more checks if the data decoded from RLP matches what we are expecting. For sure in some places we will accept a list, where binary is expected.
We do handle types, I don't think we should care about versions really, just have the latest version for the different messages.
I partially agree that we don't really need care about versions now, because whenever we make a change we remove support for old version. But I did small preparations in my code to handle different versions when that will be necessary. (You can add deserialization for different version numbers in all structures handled by Aecore.Utils.Serializable) That will come useful after minenet launch, but is not important now.
I guess there was a misunderstanding on my part here. I was talking for the versions, but maybe as you say it's not that vital to pattern-match them ATM.
@cytadela8 Maybe you could decide to either have this as low_priority or closing the issue.
I'm confused. Is this issue about adding checks that version is correct when we decode? We already do that for all Aecore.Utils.Serializable structures. Where is it missing?
I was referring to places in PeerConnection like this one. Maybe it's not needed to match it.
@gspasov @cytadela8 so we need to check rlp versions when decoding for PeerConnection?
World won't explode (nothing horrible will happen) if we don't, but it's just a simple check that should be done. If the version mismatches then we are trying to talk with a node that's incompatible with us. (that's what the version is for)
okay thanks, was unsure if this is still todo