Fixed extrinsic encoding #794
Conversation
arkpar
force-pushed
the
a-ext-encode-fix
branch
from
92f1935 to
51dfbd3
Compare
| // to use this). | ||
| let _length_do_not_remove_me_see_above: u32 = Decode::decode(input)?; | ||
| let _length_do_not_remove_me_see_above: Vec<()> = Decode::decode(input)?; |
There was a problem hiding this comment.
We are avoiding Compact<u32> here, right?
If so, the problem with Vec<()> is that the generated code is less optimal. It will actually try to check for capacity overflow, increase length and other things to keep semantics of Vec. Consider changing it to Compact.
There was a problem hiding this comment.
It does not matter as long as it does not allocate. I'd prefer not to rely on internal details of how Vec is encoded.
There was a problem hiding this comment.
Indeed, it shouldn't allocate, but still will iterate n times.
I'd prefer not to rely on internal details of how Vec is encoded.
Fair enough. The fact that it was different has lead to this issue in the first place. However, maybe the test that tests equivalence for some values would be enough?
There was a problem hiding this comment.
I've checked that Vec<()> does not do any capacity calculations. Capacity is always reported as usize::max_value() and resize function is just 6 ASM instructions in debug build, 1 inlined instruction in release build.
There was a problem hiding this comment.
Oh, interesting. That's actually makes sense, since it's ZST.
My observation was based on checking the wasm build, I wonder why it is generated this way.
pepyakin
added
A8-looksgood
and removed
A0-please_review
| // Vec<u8>. | ||
| let mut length: Vec<()> = Vec::new(); | ||
| length.resize(v.len(), ()); | ||
| length.using_encoded(|s| { v.splice(0..0, s.iter().cloned()); }); |
There was a problem hiding this comment.
will always need to do a memmove. we could skip this in most cases by a well chosen space reservation as before.
arkpar
added
A3-in_progress
arkpar
added
A0-please_review
|
Added a heuristic to reserve the prefix. |
gavofyork
added
A8-looksgood
and removed
A0-please_review
* Add LastRenominationOf Close paritytech#692 * Fix renominate tests * Build wasm
* Introduce Delta * Add VoteWeightBase * Upper bound of intention (paritytech#796) * Add UpperBoundFactor Close paritytech#672 * Check if the intention is nominating itself and add tests * Add set_upper_bound_factor and build wasm * Add LastRenominationOf (paritytech#794) * Add LastRenominationOf Close paritytech#692 * Fix renominate tests * Build wasm * genesis bitcoin (paritytech#799) genesis runtime wasm notice, do not copy wasm file into cli/src for develop! * Add MaxUnbondEntriesPerIntention (paritytech#800) * Add MaxUnbondEntriesPerIntention Close paritytech#681 * Add test * Nits * Nit * Pass tests * Replace settle_amount() with settle_and_set_amount() * Nit * Build wasm * Rename set_new_state to set_state * Rebuild wasm
* Add _at and _with methods to OnlineClient to allow for more instantiation options * tweak warnings * move a confusing comment * expose ability to set metadata and such in OnlineClient, but remove most _at methods
No description provided.