Skip to content

Protect bundle burn from adding assets with zero amount#60

Merged
PaulLaux merged 2 commits intozsa1from
validate_burning
May 29, 2023
Merged

Protect bundle burn from adding assets with zero amount#60
PaulLaux merged 2 commits intozsa1from
validate_burning

Conversation

@dmidem
Copy link

@dmidem dmidem commented May 16, 2023

This pull request addresses a specific part of a three-point task:

  1. Prevent the burning of assets with zero value.
  2. Prevent the burning of the same AssetBase more than once in a single transaction.
  3. Prevent the burning of native assets.

For points 2 and 3, the existing add_burn function in the bundle already prevents duplications and burning of native assets:

  • The function stores burns in a HashMap using the asset base as a key, preventing duplicate entries. If add_burn is called with an existing asset, its value gets added to the existing value, rather than creating a duplicate entry.
  • In addition, add_burn already verifies if the added asset is native and returns an error if that's the case, disallowing the burning of native assets.

This pull request addresses the first point: disallowing the burning of assets with zero value, a check that was previously missing.

@dmidem dmidem requested a review from PaulLaux May 16, 2023 21:45
@what-the-diff
Copy link

what-the-diff bot commented May 16, 2023

PR Summary

  • Added non-zero value check for burning
    Implemented a check to ensure that the value being burned is always greater than zero.

  • Updated tests for the new behavior
    Adjusted tests to accommodate the new change in behavior, ensuring correct functionality.

@PaulLaux PaulLaux merged commit 8e71fff into zsa1 May 29, 2023
dmidem added a commit that referenced this pull request Jun 26, 2023
Prevent the burning of assets with zero value.
@dmidem dmidem deleted the validate_burning branch August 23, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants