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

Make CALLCODE to trace value to be the code address #9881

Merged
merged 1 commit into from
Nov 26, 2018
Merged

Make CALLCODE to trace value to be the code address #9881

merged 1 commit into from
Nov 26, 2018

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Nov 7, 2018

Fixes #9878

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. labels Nov 7, 2018
@sorpaas sorpaas added this to the 2.2 milestone Nov 7, 2018
@tomusdrw tomusdrw requested a review from debris November 7, 2018 22:52
@JakeOShannessy JakeOShannessy mentioned this pull request Nov 7, 2018
Closed
tomusdrw
tomusdrw reviewed Nov 8, 2018
View reviewed changes
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good to me, but it would be good for @debris to have a look. We had some discussions about the behaviour here.

Also the release notes should mention that a re-sync is required in case of running a tracing node.

@tomusdrw tomusdrw added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Nov 8, 2018
andresilva
andresilva approved these changes Nov 9, 2018
View reviewed changes
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

LGTM. Since this changes the behavior of the trace not sure if we should backport it (although we backported this change when we did it for DELEGATECALL). Either way the changed behavior should be documented in the release notes.

@sorpaas sorpaas removed B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 labels Nov 18, 2018
@sorpaas
Copy link
Collaborator Author

sorpaas commented Nov 25, 2018

Just a note on re-syncing: nothing would break if a node does not re-sync, it would just have old tracing data using the old scheme, and new tracing data using the new scheme. So I'd say re-syncing is only required if a node really wants to fix its older data.

@sorpaas sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 25, 2018
@5chdn 5chdn modified the milestones: 2.2, 2.3 Nov 25, 2018
@5chdn 5chdn merged commit 18a2e62 into master Nov 26, 2018
@5chdn 5chdn deleted the sp-callcode-fix branch November 26, 2018 11:22
niklasad1 pushed a commit that referenced this pull request Dec 16, 2018
@sorpaas @niklasad1
Make CALLCODE to trace value to be the code address (#9881)
addbbf4
@tjayrush tjayrush mentioned this pull request Mar 25, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomusdrw tomusdrw tomusdrw left review comments

@andresilva andresilva andresilva approved these changes

@ascjones ascjones ascjones approved these changes

@debris debris Awaiting requested review from debris

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
2.3
Development

Successfully merging this pull request may close these issues.
5 participants
@sorpaas @ascjones @andresilva @tomusdrw @5chdn