Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add skip() #201

Closed
bkchr opened this issue Apr 7, 2020 · 4 comments
Closed

Add skip() #201

bkchr opened this issue Apr 7, 2020 · 4 comments
Assignees

Comments

@bkchr
Copy link
Member

bkchr commented Apr 7, 2020

The Decode trait should get a method skip(input: &mut Input) to skip an encoded object.

@bkchr bkchr added this to the SCALE codec 2.0 milestone Apr 7, 2020
@gui1117 gui1117 assigned gui1117 and unassigned gui1117 Apr 8, 2020
@gui1117
Copy link
Contributor

gui1117 commented Apr 8, 2020

we do agree that skipping an encoded object shouldn't check that the object is valid.

So if someone works on untrusted data, skipping some part of the data to get the wanted part can succeed even if the data will actually fail to decode.

@bkchr
Copy link
Member Author

bkchr commented Apr 8, 2020

We don't really check if an object is valid?

But yeah, normally we just skip the amount of expected bytes.

@gui1117
Copy link
Contributor

gui1117 commented Apr 15, 2020

Maybe we should also introduce a new function:

/// Can return the length of the type if fixed.
fn length() -> Option<usize> { None }

this would be used to optimized implementation of Vec<T>::skip and some other types

if let Some(size) = T::length {
  // decode compact and skip size*compact
}

At least we could put this new method in decode with default to None and improve on it later.

@gui1117
Copy link
Contributor

gui1117 commented Jan 22, 2021

skip is now part of Decode trait. I think we can close this issue in favor of #244
The idea is to release 2.0 soon so it can goes into next substrate release

@gui1117 gui1117 closed this as completed Jan 22, 2021
@dvdplm dvdplm mentioned this issue Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants