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

Replace this.token with methods on TokenContract #1446

Merged
merged 22 commits into from
Feb 21, 2024

Conversation

mitschabaude
Copy link
Contributor

@mitschabaude mitschabaude commented Feb 19, 2024

closes #1439

Removes this.token from SmartContract and AccountUpdate, for two reasons:

  • Reduce API bloat
  • Discourage use of token APIs on non-token contracts, because there are too many security pitfalls (for example, the main DEX example before this PR allowed arbitrary minting of liquidity tokens)

Instead, we reintroduce mint(), burn() and send() as helper methods under TokenContract.internal. The intention of this name is to make it clearer that these methods are for use within token contract methods, not from the outside (because they wouldn't be authorized when used from outside)

this.token.id is reintroduced as this.deriveTokenId() on token contracts. The reason to change the API from a property access to a function is that it's actually a function, which computes a hash in the circuit, and it's weird if a property access secretly adds constraints.

Comment on lines +55 to +58
static randomKeypair() {
let privateKey = PrivateKey.random();
return { privateKey, publicKey: privateKey.toPublicKey() };
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

quality of life improvement, it's annoying to always need two commands to create a keypair

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this example was meant as a reproduction at a time where the missing 'access' permission caused a security hole for token contracts, and is long obsolete

@mitschabaude mitschabaude changed the title Replace this.token with methods on TokenContract Replace this.token with methods on TokenContract Feb 20, 2024
@@ -71,7 +79,7 @@ class Dex extends SmartContract {
}

@method createAccount() {
this.token.mint({ address: this.sender, amount: UInt64.from(0) });
this.internal.mint({ address: this.sender, amount: UInt64.from(0) });
Copy link
Contributor

Choose a reason for hiding this comment

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

this.internal is a great way to communicate usage within smart contract methods, awesome!

src/lib/mina/token/token-contract.ts Show resolved Hide resolved
src/lib/signature.ts Show resolved Hide resolved
src/lib/zkapp.ts Outdated Show resolved Hide resolved
@mitschabaude mitschabaude merged commit 236b810 into main Feb 21, 2024
13 checks passed
@mitschabaude mitschabaude deleted the feature/move-token-helpers branch February 21, 2024 11:15
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.

Assert isNew in the deploy method of a token contract
2 participants