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

Support instant sealing #170

Merged
merged 7 commits into from
Oct 21, 2020

Conversation

ermalkaleci
Copy link
Contributor

Add support instant sealing which is useful for testing contracts.

NOTE: Although I creating a MockProvider to make waffle works, I still have to add a little delay (0.1s) between transaction to make it work but it's better then 6s

@JoshOrndorff
Copy link
Contributor

JoshOrndorff commented Oct 20, 2020

Looking good IMO @ermalkaleci Two more thoughts I'll share:

  1. I wonder if "Timestamp" should be in the name of your struct. like maybe it should be MockTimestampInherentDataProvider or something shorter, but with Timestamp included. I don't feel strongly, just sharing the idea.

  2. This is my first time encountering static mut but I did read over https://doc.rust-lang.org/reference/items/static-items.html . I wonder if using a field on the struct is a better idea and can avoid the unsafe block. Something like this:

pub struct MockTimestampInherentDataProvider {
  current_timestamp: u64,
}

@JoshOrndorff
Copy link
Contributor

Well, I guess my idea #2 above won't work because we can't mutate the instance. Maybe an alternative approach would be to read the timestamp inherent of the previous block? I'm just thinking of ways to remove the unsafe. But if @sorpaas is okay with it, I am too.

@ermalkaleci
Copy link
Contributor Author

ermalkaleci commented Oct 20, 2020

@JoshOrndorff I think unsafe is fine since this is going to be used only in testing mode. Anyway we can also do

thread_local! {
	static CURRENT_TIMESTAMP: RefCell<u64> = RefCell::new(0);
}

and get rid-off unsafe

Copy link
Member

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

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

LGTM! Just a minor nit.

@sorpaas sorpaas merged commit 7b836a2 into polkadot-evm:master Oct 21, 2020
@ermalkaleci ermalkaleci deleted the instant_sealing branch October 21, 2020 17:40
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
* support both manual and instant sealing

* fix name

* remove sealing.unwrap()

* make MockTimestamp to start from 0

* update docs

* use thread_local to avoid unsafe

* remove expect
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.

3 participants