nav-io / navio-core

Navio Core v8 [WIP]
https://nav.io
MIT License
3 stars 6 forks source link

p2p: update protocol magic bytes #9

Closed aguycalled closed 2 years ago

aguycalled commented 2 years ago

Each network (mainnet, testnet, regnet) needs to get their protocol magic bytes updated to unique Navcoin values.

Ideally they should not be equal to other bitcoin-based networks, but for the sake of simplicity, they can be set randomly.

isgulkov commented 2 years ago

So, this is pchMessageStart in src/chainparams.cpp.

As this is a change in the network protocol, we'll also need to support the old values (here and perhaps somewhere else) until old versions are phased out

Ideally they should not be equal to other bitcoin-based networks, but for the sake of simplicity, they can be set randomly

Random values are good enough — 4 bytes is too much entropy for any collision to occur, realistically speaking. One caveat is that the randomness should be good. So, what I'd do:

  1. Run this on a computer with hardware RNG:

    $ hexdump -n 12 -e '4/1 "%02x " "\n"' /dev/urandom
    11 56 a6 9d
    05 c9 85 bd
    89 a7 7f 85
  2. Reject values according to the original spec:

    /**
    * The message start string is designed to be unlikely to occur in normal data.
    * The characters are rarely used upper ASCII, not valid as UTF-8, and produce
    * a large 32-bit integer with any alignment.
    */
  3. Run each one through Google search

aguycalled commented 2 years ago

Thanks Ilya for your observation and your careful approach. In our specific case we don't need to keep backwards compatibility with old nodes as we are starting a new chain. And essentially, we want to make old clients incompatible with the new one so their interactions are prevented.

isgulkov commented 2 years ago

Thanks Ilya for your observation and your careful approach. In our specific case we don't need to keep backwards compatibility with old nodes as we are starting a new chain.

Oh, OK. And what we have in the master branch is the new, incompatible version we're working on, right?

aguycalled commented 2 years ago

Yes, exactly.

mxaddict commented 2 years ago

Since the code changes for this were already merged, I think we should be closing this issue?