-
Notifications
You must be signed in to change notification settings - Fork 14
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
fixing benchmarks #264
fixing benchmarks #264
Conversation
FYI @clangenb |
@@ -15,7 +15,6 @@ | |||
|
|||
*/ | |||
|
|||
#![cfg_attr(not(feature = "std"), no_std)] | |||
pub extern crate alloc; |
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 line can also be removed.
@@ -117,7 +129,7 @@ pub type Moment = u64; | |||
|
|||
impl pallet_timestamp::Config for Test { | |||
type Moment = Moment; | |||
type OnTimestampSet = (); | |||
type OnTimestampSet = Aura; |
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.
Hmm, I see. This whole PR looks like an overkill, but I guess we can remove the feature flag now in our runtimes at least 🥳
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.
it was more involved than I hoped, but I saw no other way to make the benchmarks work. Was there a better way? What feature flag do you mean?
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.
Uff, I see it is gone now: https://github.com/integritee-network/integritee-node/blob/1a18046482c72620898300e04b29fa7a72e7a3ff/runtime/src/lib.rs#L289
Here we used to have a feature flag that set OnTimestamp= ()
in case we were running the benchmark because Aura didn't like it when we messed with time. But I guess your implementation now, is better than the runtime hack we did before anyhow.
our benchmarks failed with mismatch in slot to timestamp.
this fixes it