Add preview net ID #5396
Add preview net ID #5396
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## feature/stable-cadence #5396 +/- ##
==========================================================
- Coverage 56.33% 56.31% -0.02%
==========================================================
Files 1023 1023
Lines 99105 99116 +11
==========================================================
- Hits 55827 55819 -8
- Misses 39059 39073 +14
- Partials 4219 4224 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } else { | ||
| sameChainCheck := chain.IsValid(address) | ||
| require.True(t, sameChainCheck) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // sanity check: mainnet addresses must fail the test check | ||
| r = uint64(rand.Intn(maxIndex - loop)) | ||
| for k := 0; k < loop; k++ { |
There was a problem hiding this comment.
removed as this is covered by the test section just above
|
I checked that the systemcontracts are properly set up for this, and they are. |
| @@ -273,6 +273,9 @@ const invalidCodeTransientNetwork = uint64(0x1cb159857af02018) | |||
| // invalidCodeSandboxNetwork is the invalid codeword used for Sandbox network. | |||
| const invalidCodeSandboxNetwork = uint64(0x1035ce4eff92ae01) | |||
|
|
|||
| // invalidCodePreviewNetwork is the invalid codeword used for Preview networks. | |||
There was a problem hiding this comment.
Still need invalidCodePreviewNetwork if we actually do need a brand new chain ID. Typically, this is something i'd get @tarakby's help with.
Can we add comment how this codeword is produced? or is there a test function to produce or validate this value?
There was a problem hiding this comment.
added more comments here: f627367.
Understanding all the details require going through the other comments in the implementation, and reading the basics about Hamming codes.
There was a problem hiding this comment.
From my layman understanding, I think for generation it is something like this:
while
r = rand_uint64()
if r != 0 and not isValidCodeWord(r):
break
There was a problem hiding this comment.
Correct, though isValidCodeWord(r) should be applied to all existing networks (Mainnet and all others). I also choose r with a hamming weight close to 32 so that we distance the test space from the Mainnet space
There was a problem hiding this comment.
@tarakby off topic but I was wondering that recently, is it possible to write a function that suggests closest address ( when user writes the address wrong ) something like did you mean xxxxxx ?
There was a problem hiding this comment.
@tarakby Could you maybe add this "layman understanding" to the comment? It helped me understand what is happening there :-)
There was a problem hiding this comment.
is it possible to write a function that suggests closest address ( when user writes the address wrong )
Good question! the scheme was designed only for error detection (up to 6 bits) and not error correction. However I speculate the scheme can also provide up to 3 bits of error correction (no proof, only gut feeling), which means we can guess the original address correctly if we believe up to 3 bits only got flipped.
If more bits were flipped, we can still make a guess, but the guess will be wrong (different than the original address), which is dangerous from a blockchain user perspective (imagine sending tokens to the wrong address because it was suggested by an interface). I think it's safer to only check the address and reject any invalid address, to force the user to double check.
There was a problem hiding this comment.
Could you maybe add this "layman understanding" to the comment? It helped me understand what is happening there
makes sense, and it's useful for future extensions of the code. I'll add it in another PR
…into kan/preview-net-chain-id
Co-authored-by: Bastian Müller <bastian@turbolent.com>
Still need invalidCodePreviewNetwork if we actually do need a brand new chain ID. Typically, this is something i'd get @tarakby's help with.
Otherwise, this is more or a less looking off of this PR: https://github.com/onflow/flow-go/pull/3433/files