Skip to content

fix: Fix crash in Actor.push_data with PPE and a strict charging limit #664

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 6, 2025
Merged

fix: Fix crash in Actor.push_data with PPE and a strict charging limit #664

merged 1 commit into from
Nov 6, 2025

Conversation

@janbuchar
Copy link
Contributor

@janbuchar janbuchar commented Nov 6, 2025

@janbuchar janbuchar added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Nov 6, 2025
@github-actions github-actions bot added this to the 127th sprint - Tooling team milestone Nov 6, 2025
@janbuchar janbuchar changed the title fix: Fix crash push_data with PPE and a strict charging limit fix: Fix crash in Actor.push_data with PPE and a strict charging limit Nov 6, 2025
Mantisus
Mantisus approved these changes Nov 6, 2025
View reviewed changes
Copy link
Collaborator

@Mantisus Mantisus left a comment

Choose a reason for hiding this comment

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

LGTM

Pijukatel
Pijukatel approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@Pijukatel Pijukatel left a comment

Choose a reason for hiding this comment

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

It would be nice to add test for the combination of inputs that caused the bug.

vdusek
vdusek approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@vdusek vdusek left a comment

Choose a reason for hiding this comment

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

LGTM

@vdusek vdusek merged commit 8f2e4b2 into master Nov 6, 2025
39 of 41 checks passed
@vdusek vdusek deleted the fix-ppe-push-data branch November 6, 2025 12:18
@janbuchar
Copy link
Contributor Author

janbuchar commented Nov 6, 2025

It would be nice to add test for the combination of inputs that caused the bug.

the steps to reproduce it were basically to run an Actor with a spending limit that makes it impossible to charge even a single occurence of an event and then call push_data with that event. This triggers a push_data call with count=0. And now that I think of it, this may not have resolved the actual reported issue 😁

@janbuchar janbuchar mentioned this pull request Nov 6, 2025
Merged
janbuchar added a commit that referenced this pull request Nov 6, 2025
@janbuchar
fix: Avoid charge calls with count=0 (#665)
a0f894e 
- follow up to #664
- I haven't managed to reproduce the actual bug with a test yet, so that
will happen later
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdusek vdusek vdusek approved these changes

@Pijukatel Pijukatel Pijukatel approved these changes

+1 more reviewer

@Mantisus Mantisus Mantisus approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@janbuchar janbuchar

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Milestone

127th sprint - Tooling team

Development

Successfully merging this pull request may close these issues.

5 participants

@janbuchar @vdusek @Mantisus @Pijukatel