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

Implement concurrent minting/burning #55

Merged
merged 8 commits into from
May 8, 2024
Merged

Conversation

kantp
Copy link
Collaborator

@kantp kantp commented May 7, 2024

In order to do this, we remove the total supply (and the check that the circulating supply does not exceed it) from the main contract, and use actions/reducers to update the circulating supply.

Enforcing a limit on the circulating supply will then be the responsibility of the admin contract.

kantp added 2 commits May 6, 2024 16:18
Using inheritance to change which constructor is used does not work. Instead, we are using a
`static` field, which can be set without changing the class.
@kantp kantp force-pushed the kantp/concurrent-minting branch from b05c744 to 8ea2b7a Compare May 7, 2024 12:15
@kantp kantp force-pushed the kantp/concurrent-minting branch from 529cf28 to 533241d Compare May 7, 2024 15:04
kantp added 4 commits May 8, 2024 11:34
Some tests that assert that something that should not work does indeed not work will fail when we
disable proofs in the tests. So we should skip them if `proofsEnabled` is false.
@kantp kantp force-pushed the kantp/concurrent-minting branch from cb23a26 to 8542c20 Compare May 8, 2024 11:12
@kantp kantp marked this pull request as ready for review May 8, 2024 11:13
@kantp kantp requested a review from harrysolovay as a code owner May 8, 2024 11:13
@kantp kantp requested review from qwadratic and removed request for harrysolovay May 8, 2024 11:13
Copy link
Collaborator

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

nice

FungibleToken.ts Outdated Show resolved Hide resolved
FungibleToken.ts Outdated Show resolved Hide resolved
Comment on lines +611 to +613
it("should not mint too many B tokens using the vanilla admin contract", {
skip: !proofsEnabled,
}, async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why this is skipped when proofs are disabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question! I observed that this test fails when proofs are disabled.

My suspicion is that when proofs are disabled, the verification key of the admin contract is never checked, so you can get away with switching the admin contract. And this test tests that that is prevented (which only works when the proofs are actually constructed and checked).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, that makes sense

@kantp kantp merged commit 1fdd3a6 into main May 8, 2024
3 checks passed
@kantp kantp deleted the kantp/concurrent-minting branch May 8, 2024 16:26
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