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

[contracts] Make debug buffer work like a FIFO pipe #12953

Merged
merged 6 commits into from
Dec 27, 2022
Merged

Conversation

agryaznov
Copy link
Contributor

@agryaznov agryaznov commented Dec 16, 2022

Follow-up to #12845

Instead of falling on the debug buffer exhaustion, drain it from the beginning by the overflowing message size, effectively making it a FIFO buffer.

This should make the contract outcome the same for an on-chain run (where debug buffer normally disabled, and hence execution can't fail with its exhaustion) and an RPC call (often having it enabled for debugging, hence currently the same contract could fail here while succeed on-chain).

@agryaznov agryaznov added A0-please_review Pull request needs code review. I3-bug The node fails to follow expected behavior. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 16, 2022
@agryaznov agryaznov requested a review from athei as a code owner December 16, 2022 13:40
@agryaznov agryaznov requested a review from xermicus December 16, 2022 13:41
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

frame/contracts/src/exec.rs Outdated Show resolved Hide resolved
frame/contracts/src/exec.rs Show resolved Hide resolved
@athei athei added D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 27, 2022
@athei athei merged commit 8d20ed5 into master Dec 27, 2022
@athei athei deleted the ag-debug-buf-truncate branch December 27, 2022 13:24
rossbulat pushed a commit that referenced this pull request Dec 28, 2022
* make debug buffer work like a FIFO pipe

* remove unused Error type

* Remove panics

* Update frame/contracts/src/exec.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 19, 2023
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* make debug buffer work like a FIFO pipe

* remove unused Error type

* Remove panics

* Update frame/contracts/src/exec.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* make debug buffer work like a FIFO pipe

* remove unused Error type

* Remove panics

* Update frame/contracts/src/exec.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited I3-bug The node fails to follow expected behavior.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants