-
Notifications
You must be signed in to change notification settings - Fork 112
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
runtime: Fix verification of blocks between two trusted heights #5134
Conversation
3c26dd5
to
bff9daf
Compare
bff9daf
to
ec95484
Compare
Codecov Report
@@ Coverage Diff @@
## master #5134 +/- ##
==========================================
- Coverage 66.99% 66.76% -0.24%
==========================================
Files 496 496
Lines 53275 53275
==========================================
- Hits 35692 35567 -125
- Misses 13255 13352 +97
- Partials 4328 4356 +28
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Running cargo update
shows some packages were just updated.
[[package]]
name = "clap"
-version = "4.0.32"
+version = "4.1.1"
[[package]]
name = "clap_lex"
-version = "0.3.0"
+version = "0.3.1"
/// runtime's untrusted local store fails. | ||
/// | ||
pub fn save(&self, store: &Box<dyn LightStore>) { | ||
let lowest_block = store.lowest(Status::Trusted).unwrap(); |
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 panic here? As load
can return an empty vector of trusted blocks, I would also allow that in save
. We can pass trust_root
to this function and use it when the highest block is not available.
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.
This would never panic in practice as the light store always has at least one block (e.g. at least the trust root added during initialization).
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 know, just thought it would be nice to remove panics.
ec95484
to
08ccab5
Compare
Updated, please take another look. |
08ccab5
to
5b03671
Compare
Updates tendermint-rs to include our fix from informalsystems/tendermint-rs#1247.