dfinity / cycles-wallet

DFINITY Cycles Wallet
Apache License 2.0
55 stars 30 forks source link

fix: use ByteBuf to store internal wasm #84

Closed p-shahi closed 3 years ago

p-shahi commented 3 years ago

use ByteBuf (to store internal wasm) to fix hitting cycle limit in single message on upgrade

nomeata commented 3 years ago

I guess would be nice if Candid would be as efficient with Vec<u8> than with ByteBuf. Is that possible, @chenyan-dfinity ?

chenyan-dfinity commented 3 years ago

No, it's not possible. That would require the specialization RFC for Rust, see https://github.com/rust-lang/rust/issues/31844

nomeata commented 3 years ago

You can probably do the same trick that Haskell does with Show, where [Char] serializes differently (namely than a string) than [t] for other t. Given the pervasiveness of Vec, it might be worth it?

Haskell’s Show type class has (simplified) show :: a -> String, but also showList :: [a] -> String. Most implementations of the class don’t do anything special for showList but use the default implementation, but this way, the instance for Char can do something special here. And the instance for [a] has show = showList.

p-shahi commented 3 years ago

Maybe this discussion is better had in the candid repo (in a issue/feature)? Might be helpful to track the history/life of this discussion there than here.

chenyan-dfinity commented 3 years ago

Yes, that's the exactly the specialization RFC. You need both impl Deserialize for Vec<u8> and impl Deserialize for Vec<T>, which is not allowed by Rust.

nomeata commented 3 years ago

No, you misread me. There is no instance Show [Char] in Haskell either!

Will open an issue

nomeata commented 3 years ago

https://github.com/dfinity/candid/issues/224