-
Notifications
You must be signed in to change notification settings - Fork 765
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
Store refund counter in EVM #612
Conversation
Hm, should we create separate PRs for istanbul bugfixes or do everything in #607? the thing is that if we create separate PRs, the CI will be failing for them (as other bugs are still there). |
@s1na Think it makes sense to keep it towards #607 since there would be various things to address before CI will pass through again. Not sure if its realistic, but can we optimally very much prioritize here (Sina, could you eventually take some short-term extra time)? The HF on mainnet is in 3-4 weeks and if somehow possible we should get out a release out until mid next week? @evertonfraga Can you also eventually also jump in here a bit? If you are not deep enough in the VM code yet some preparatory documentation work would also help very much, e.g. an analysis together with some listing which tests are actually failing and which related EIPs are affected, eventually you can also already dig in one level deeper and investigate what constellation these tests are actually testing. //cc @alcuadrado 1-2 extra days as well? 🙂 Still have to ask you on your current team status, will continue conversation on Gitter. |
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.
Looks good, thanks!
@@ -33,7 +33,6 @@ export interface Env { | |||
export interface RunResult { | |||
logs: any // TODO: define type for Log (each log: [Buffer(address), [Buffer(topic0), ...]]) | |||
returnValue?: Buffer | |||
gasRefund: BN |
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.
Wouldn't this introduce a breaking change? I think we should duplicate the gasRefund
value for now, and add a comment about this field being deprecated and removed in the future.
Is this possible @s1na? I'm not completely sure atm.
@holgerd77 sure thing |
To be merged into #607
Fixes #611