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

check max_supply before borrowing MPAs #1498

Merged
merged 9 commits into from
Jan 25, 2019
Merged

check max_supply before borrowing MPAs #1498

merged 9 commits into from
Jan 25, 2019

Conversation

jmjatlanta
Copy link
Contributor

Fixes #1465

Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

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

What about existing MPAs and their max_supply?
I think we need hardfork protection on the new FC_ASSERT and some logic to adapt max_supply of existing bitAssets at hardfork time, if necessary.

@jmjatlanta
Copy link
Contributor Author

and some logic to adapt max_supply of existing bitAssets at hardfork time, if necessary.

How do you see this working? My thought: At the hardfork, flip through all bitAssets, if current supply is greater than max supply, adjust max supply to equal current supply. What if that number is greater than GRAPHENE_MAX_SHARE_SUPPLY?

@pmconrad
Copy link
Contributor

pmconrad commented Jan 2, 2019

Shouldn't happen due to softfork protection. If it does happen set max_supply to GRAPHENE_MAX_SHARE_SUPPLY.

@pmconrad
Copy link
Contributor

pmconrad commented Jan 8, 2019

Test still failing because after generate_blocks, alice is no longer a valid reference (in line 2057).

@jmjatlanta
Copy link
Contributor Author

Test still failing because after generate_blocks, alice is no longer a valid reference (in line 2057).

Gee wiz. It passed for me. The problem must be on your end! :-D

Fixed reference, tested in Ubuntu 18.10 in both Debug and Release mode.

@jmjatlanta jmjatlanta dismissed pmconrad’s stale review January 15, 2019 20:52

Hardfork protection added

pmconrad
pmconrad previously approved these changes Jan 17, 2019
Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

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

Thanks!

libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
cogutvalera
cogutvalera previously approved these changes Jan 19, 2019
Copy link
Member

@cogutvalera cogutvalera left a comment

Choose a reason for hiding this comment

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

Thanks

@jmjatlanta jmjatlanta merged commit 0f10387 into hardfork Jan 25, 2019
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.

4 participants