Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fixing delegatecall #196

Merged
merged 9 commits into from
Jan 25, 2016
Merged

Fixing delegatecall #196

merged 9 commits into from
Jan 25, 2016

Conversation

tomusdrw
Copy link
Collaborator

I'm not sure about the decision to extend Ext interface and adding ActionValue to ActionParams.
Let me know what you guys think.

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Jan 20, 2016
@gavofyork
Copy link
Contributor

would have preferred to see an additional param to call rather than a whole new function delegatecall. see e.g. how CALLCODE is done.

@tomusdrw
Copy link
Collaborator Author

@gavofyork Fixed.

@@ -566,34 +566,65 @@ impl Interpreter {
}
};
},
instructions::CALL | instructions::CALLCODE | instructions::DELEGATECALL => {
assert!(ext.schedule().call_value_transfer_gas > ext.schedule().call_stipend, "overflow possible");
instructions::DELEGATECALL => {
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a lot of duplicated code here. merge with the next arm, perhaps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 23, 2016
gavofyork pushed a commit that referenced this pull request Jan 25, 2016
@gavofyork gavofyork merged commit f014a1f into master Jan 25, 2016
@gavofyork gavofyork deleted the delegatecall branch January 25, 2016 23:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-grumble 🔥 Pull request has minor issues that must be addressed before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants