-
Notifications
You must be signed in to change notification settings - Fork 254
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
chore(deps): Update smoldot to the latest version #1400
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
// non_finalized_headers_subscription(&api).await?; | ||
finalized_headers_subscription(&api).await?; | ||
runtime_api_call(&api).await?; | ||
storage_plain_lookup(&api).await?; | ||
dynamic_constant_query(&api).await?; | ||
dynamic_events(&api).await?; | ||
// runtime_api_call(&api).await?; | ||
// storage_plain_lookup(&api).await?; | ||
// dynamic_constant_query(&api).await?; | ||
// dynamic_events(&api).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these still be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I've added a few debug logs in f14bf9a.
This is to help reproduce the issue from smoldot faster smol-dot/smoldot#1638.
I suggest putting this PR on pending until another smoldot version is published on crates io :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a plan :)
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
size This reverts commit 74dd9d7. Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lightclient/src/background.rs
Outdated
fn trim_response(response: &str) -> &str { | ||
const MAX_RESPONSE_SIZE: usize = 256; | ||
let len = std::cmp::min(response.len(), MAX_RESPONSE_SIZE); | ||
&response[..len] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, you could actually index into non-char boundary here because not all chars are ascii which leads to a panic..
https://github.com/paritytech/jsonrpsee/blob/master/core/src/tracing.rs#L101-#L124
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a couple of nightly only methods that would be perfect here, but in the absense of them one could do:
while !response.is_char_boundary(len) {
len -= 1;
}
&response[..len]
To find the appropriate place to slice a message that exceeds the length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've ended up with something similar to Niklas's suggestion, I found that a bit easier to follow :D
lightclient/src/background.rs
Outdated
let (maybe_truncated, delim) = if back_message.len() > MAX_LOG_SIZE { | ||
(&back_message[0..MAX_LOG_SIZE], "...") | ||
} else { | ||
(&back_message[..], "") | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to worry about invalid UTF boundaries here too?
(should trim_repsonse
take in a max length and then it can be used here too?)
/// Use the unstable backend for testing. | ||
const USE_UNSTABLE_BACKEND: bool = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it makes sense to test the light client with just the "unstable" backend, though we could have that light client test be a fn that takes in this bool and then run it twice to test both backends until the unstable one is stabilised? (and then remove this const)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of small nits but looks great to me; thanks for doing this!
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
This PR updates smoldot to 0.17 and smoldot-light to 0.15.
Changes to repo:
Pending fix from:
chain_subscribeFinalizedHeads
subscription seems to not produce blocks smol-dot/smoldot#1638Status Update 8 July
Pending fix: