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

Update dfinity #315

Closed
wants to merge 5 commits into from
Closed

Update dfinity #315

wants to merge 5 commits into from

Conversation

eftychis
Copy link
Contributor

No description provided.

impl str::FromStr for CanisterId {
type Err = num::ParseIntError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(CanisterId::from_u64(u64::from_str(s)?))
Ok(CanisterId(Blob(s.to_string().into_bytes())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not in this PR, but what is the semantics of from_str? A str is text, never binary data (“String slices are always valid UTF-8.” according to the docs), so I guess this trait should either decode the the textual representation, or maybe simply not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point and I was planning to fix it actually here, after other fires in this PR get resolved. I will continue working on this today most likely. If it ends up being to much work I will file it as a high priority bug (due to how hard it will be to debug) and defer for follow up.

@basvandijk
Copy link
Contributor

Could dfinity be updated to at least https://github.com/dfinity-lab/dfinity/commit/07ed5bbf4681deca7882e98c3f67b15fe0b82597?

I need this to get restrict-eval support which is needed to fix a security vulnerability in our CI system.

@eftychis
Copy link
Contributor Author

This was done in #336, #337 and #346.

@eftychis eftychis closed this Jan 30, 2020
@eftychis
Copy link
Contributor Author

hat trick ;)

@lwshang lwshang deleted the eftychis-update-dfinity-breaking branch July 29, 2022 18:58
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 this pull request may close these issues.

None yet

3 participants