Polkadex-Substrate / polkadexTEE-worker

Polkadex Off-chain Orderbook
Apache License 2.0
10 stars 1 forks source link

Change string conversions to encode() #96

Open haerdib opened 3 years ago

haerdib commented 3 years ago

We somehow need to ensure that when comparing Strings in their UTF8 representation, all strings are converted the same way, either by .encode() from parity scale codec or .as_bytes(). But never mixed. They are not compatible:

When testing within the enclave:

let string = "hello_world".to_string();
let string_test_codec = string.encode();
let string_test_uft8 = string.as_bytes();
assert_eq!(string_test_codec, string_test_uft8);

The test failed:

thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `[44, 104, 101, 108, 108, 111, 95, 119, 111, 114, 108, 100]`,
 right: `[104, 101, 108, 108, 111, 95, 119, 111, 114, 108, 100]`',

Because with the full feature of parity scale codec the String::decode() is now working within the enclave (see this issue for more information) I think it is easier to ensure no as_bytes() is used compared to no encode() for string. Mainly because encode() is widely used for many different types and structs, while as_bytes() is not.

haerdib commented 3 years ago

Actually, that does not work. All Strings sent to and received from openfinex need to be handled with normal from_uft8() and as_bytes(). Or atleast some conversion mechanism needs to be built.

haerdib commented 3 years ago

I'm moving this issue to the main net. This should not be done hurriedly and it would be best to use this in combination with integration tests.. otherwise one might break alot of things.

haerdib commented 3 years ago

With the removal of openfinex, this should not be an issue anymore, as we can completely remove the normal uft8 conversion. We'll need to see what the future brings about, I guess..