-
Notifications
You must be signed in to change notification settings - Fork 283
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
[master] Remote State Reads #2452
Conversation
I've looked through the changes, and apart from the version issue it passes the smell test. I don't know enough about the blockchain layer to do a proper review, though. |
Ping !. I have some more changes pending that I need to do on top of this change after its merged. So it would be nice to have this merged at the earliest. |
@@ -579,6 +580,7 @@ bool AccountStoreSC<MAP>::UpdateAccounts(const uint64_t& blockNum, | |||
|
|||
if (ret && !ParseCallContract(gasRemained, runnerPrint, receipt, | |||
tree_depth, scilla_version)) { | |||
Contract::ContractStorage2::GetContractStorage().RevertPrevState(); |
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.
may not need this since it called later at line 588.
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.
agree
@@ -98,6 +98,7 @@ bool SerializeToArray(const T& protoMessage, bytes& dst, | |||
string ContractStorage2::GenerateStorageKey(const dev::h160& addr, | |||
const string& vname, | |||
const vector<string>& indices) { | |||
LOG_MARKER(); |
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.
can be removed?
@@ -579,6 +580,7 @@ bool AccountStoreSC<MAP>::UpdateAccounts(const uint64_t& blockNum, | |||
|
|||
if (ret && !ParseCallContract(gasRemained, runnerPrint, receipt, | |||
tree_depth, scilla_version)) { | |||
Contract::ContractStorage2::GetContractStorage().RevertPrevState(); |
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.
agree
b30f0e4
to
ede0c3f
Compare
Description
This is a redo of #2167 without the MPT / Trie parts.
Backward Compatibility
Review Suggestion
Status
It is ready for review, but requires more testing. I might need @kaikawaliu's and @renlulu's help for that.
Implementation
Integration Test (Core Team)