Skip to content
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

⚡️ Add check for allocation smaller than decimals #280

Merged
merged 1 commit into from
May 2, 2024
Merged

⚡️ Add check for allocation smaller than decimals #280

merged 1 commit into from
May 2, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented May 1, 2024 •
edited
Loading

What?

  • Reject project metadatas on creation and edit where the allocation size is smaller than the specified decimals.

Why?

  • It introduces memory leaks, which would behave the same as panics and stall our chain

How?

  • add a new check on the metadata validation function

Testing?

allocation_smaller_than_decimals on application test file

This was referenced May 1, 2024
Merged
Merged
Merged
Merged
@JuaniRios Graphite App
Copy link
Contributor Author

JuaniRios commented May 1, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @JuaniRios and the rest of your teammates on Graphite Graphite

@JuaniRios JuaniRios marked this pull request as ready for review May 1, 2024 13:24
@graphite-app Graphite App
Copy link

graphite-app bot commented May 1, 2024

Graphite Automations

"Auto-assign PRs to author" took an action on this PR • (05/01/24)

1 assignee was added to this PR based on Juan Ignacio Rios's automation.

@JuaniRios JuaniRios requested review from lrazovic and vstam1 May 1, 2024 13:25
@JuaniRios JuaniRios changed the title add check for allocation smaller than decimals ⚡️ Add check for allocation smaller than decimals May 1, 2024
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from d26afe5 to 421bd82 Compare May 1, 2024 13:26
vstam1
vstam1 approved these changes May 2, 2024
View reviewed changes
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

Do you know why it introduces memory leaks?

@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from b0b4644 to 4990206 Compare May 2, 2024 07:53
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 421bd82 to 2eeb943 Compare May 2, 2024 07:53
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 4990206 to 8401d91 Compare May 2, 2024 07:57
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 2eeb943 to 7e6273c Compare May 2, 2024 07:57
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 8401d91 to 13c0fec Compare May 2, 2024 07:58
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 7e6273c to e514af9 Compare May 2, 2024 07:58
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 13c0fec to 8addb0d Compare May 2, 2024 07:59
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from e514af9 to 825c882 Compare May 2, 2024 07:59
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 8addb0d to 7f7e83c Compare May 2, 2024 08:01
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 825c882 to 08be85e Compare May 2, 2024 08:01
@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 7f7e83c to 97e99f9 Compare May 2, 2024 08:13
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 08be85e to 6efab70 Compare May 2, 2024 08:13
@JuaniRios Graphite App
Copy link
Contributor Author

Do you know why it introduces memory leaks?

Nope, but it's reproducible

@JuaniRios JuaniRios requested a review from vstam1 May 2, 2024 08:15
@JuaniRios Graphite App
Copy link
Contributor Author

JuaniRios commented May 2, 2024 •
edited
Loading

Merge activity

  • May 2, 5:12 AM EDT: @JuaniRios started a stack merge that includes this pull request via Graphite.
  • May 2, 5:21 AM EDT: Graphite rebased this pull request as part of a merge.
  • May 2, 5:22 AM EDT: @JuaniRios merged this pull request with Graphite.

@JuaniRios JuaniRios force-pushed the 05-01-accept_only_ct_decimals_from_4_to_20 branch from 97e99f9 to edd2894 Compare May 2, 2024 09:18
Base automatically changed from 05-01-accept_only_ct_decimals_from_4_to_20 to main May 2, 2024 09:20
@JuaniRios JuaniRios force-pushed the 05-01-add_check_for_allocation_smaller_than_decimals branch from 6efab70 to 9f32577 Compare May 2, 2024 09:20
@JuaniRios JuaniRios merged commit 943ed38 into main May 2, 2024
@JuaniRios JuaniRios deleted the 05-01-add_check_for_allocation_smaller_than_decimals branch May 2, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrazovic lrazovic Awaiting requested review from lrazovic

@vstam1 vstam1 Awaiting requested review from vstam1

Assignees

@JuaniRios JuaniRios

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JuaniRios @vstam1