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.

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

fix: update RandomnessLotteryDemo TODOs #3139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VolodymyrBg
Copy link

What does it do?

Fixes two TODO items in the RandomnessLotteryDemo contract:

  1. Sets a proper gas limit (200,000) for fulfillment operations, which is sufficient for processing random words and transferring rewards
  2. Replaces hardcoded deposit value with dynamic contract call to RANDOMNESS_CONTRACT.requiredDeposit()

What important points reviewers should know?

  • The gas limit of 200,000 was chosen to safely cover all operations: processing random words, calculating winners, transferring ETH, and emitting events
  • Using requiredDeposit() makes the contract more maintainable as it will automatically adapt if deposit requirements change

Is there something left for follow-up PRs?

No, all TODOs in this file have been addressed.

What alternative implementations were considered?

For the gas limit:

  • Lower values were considered but might be insufficient for all operations
  • Higher values would unnecessarily increase transaction costs

For the deposit:

  • The hardcoded value was replaced with a dynamic call to ensure the contract stays up-to-date with any deposit requirement changes

Are there relevant PRs or issues in other repositories?

No, these changes are self-contained within the Moonbeam repository.

What value does it bring to the blockchain users?

  1. More reliable lottery contract with properly sized gas limits
  2. Better maintainability by using dynamic deposit requirements instead of hardcoded values

- Set safe gas limit (200,000) for fulfillment operations
- Replace hardcoded deposit with actual contract call
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.

1 participant