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

RSKIP197 - Fix Precompile Calls Not Conforming With CALL Semantics #197

Merged
merged 7 commits into from Jul 16, 2021

Conversation

fedejinich
Copy link
Contributor

@fedejinich fedejinich commented Dec 15, 2020

Introducing a new mechanism to handle errors on failed precompiled calls

@SergioDemianLerner SergioDemianLerner added the Incomplete Incomplete Proposals label Apr 5, 2021
@fedejinich fedejinich closed this Jun 1, 2021
@fedejinich fedejinich reopened this Jun 1, 2021
@fedejinich fedejinich marked this pull request as ready for review July 12, 2021 21:04
@fedejinich
Copy link
Contributor Author

I've just reopened this RSKIP with the specification @SergioDemianLerner @nagarev , any comments?

IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
IPs/RSKIP197.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@SergioDemianLerner SergioDemianLerner left a comment

Choose a reason for hiding this comment

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

As far as I understand this change, the RSKIPS should have a single line and nothing more:

"Add REVERT capability to pre-compiled contracts"

That's all that is needed to understand the change. There is no need for examples.

@SergioDemianLerner
Copy link
Collaborator

I'm still trying to understand why this is a consensus change. The only reason I found is that this change might bring some unintended side-effects, so we synchronize the activation of this change with the Iris upgrade.

@fedejinich
Copy link
Contributor Author

fedejinich commented Jul 13, 2021

I'm still trying to understand why this is a consensus change. The only reason I found is that this change might bring some unintended side-effects, so we synchronize the activation of this change with the Iris upgrade.

Thats right, we are trying to synchronize some unexpected behavior from rskj that might affect a transaction final state. Do you think there's no need to include it as RSKIP?

Maybe we can do some sync-tests to validate a safe upgrade

@fedejinich
Copy link
Contributor Author

fedejinich commented Jul 13, 2021

here is a much simplified version of this RSKIP, a oneliner

@fedejinich fedejinich changed the title RSKIP197 - Error Handling for Precompiled Contracts RSKIP197 - Add REVERT Capability to Pre-Compiled Contracts Jul 13, 2021
@fedejinich fedejinich force-pushed the precompiled_contracts_error_handling branch from fbfd08c to 45a340f Compare July 14, 2021 20:04
@fedejinich fedejinich changed the title RSKIP197 - Add REVERT Capability to Pre-Compiled Contracts RSKIP197 - Fix Precompile Calls Not Conforming With CALL Semantics Jul 16, 2021
@fedejinich fedejinich force-pushed the precompiled_contracts_error_handling branch from 45a340f to 78e580a Compare July 16, 2021 17:04
@SergioDemianLerner SergioDemianLerner merged commit 104886c into master Jul 16, 2021
@delete-merged-branch delete-merged-branch bot deleted the precompiled_contracts_error_handling branch July 16, 2021 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Incomplete Incomplete Proposals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants