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

fixing benchmarks #264

Merged
merged 7 commits into from
Jul 20, 2024
Merged

fixing benchmarks #264

merged 7 commits into from
Jul 20, 2024

Conversation

brenzi
Copy link
Contributor

@brenzi brenzi commented Jul 19, 2024

our benchmarks failed with mismatch in slot to timestamp.
this fixes it

@brenzi brenzi requested a review from clangenb July 20, 2024 07:23
@brenzi
Copy link
Contributor Author

brenzi commented Jul 20, 2024

FYI @clangenb

@brenzi brenzi merged commit 5670f54 into master Jul 20, 2024
12 checks passed
@@ -15,7 +15,6 @@

*/

#![cfg_attr(not(feature = "std"), no_std)]
pub extern crate alloc;
Copy link
Contributor

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;
Copy link
Contributor

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 🥳

Copy link
Contributor Author

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?

Copy link
Contributor

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.

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.

2 participants