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

The parameters for burn are redundant #15

Open
PaulRBerg opened this issue Jul 24, 2024 · 0 comments
Open

The parameters for burn are redundant #15

PaulRBerg opened this issue Jul 24, 2024 · 0 comments
Labels
effort: high Large or difficult task. priority: 1 This is important. It should be dealt with shortly. type: refactor Change that neither fixes a bug nor adds a feature. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.

Comments

@PaulRBerg
Copy link
Member

Copy-pasting the criticism shared by @IaroslavMazur here:

Note that because:

  • we need to make sure the burner should, actually, be allowed to burn the tokens and
  • we won't have approvals,
    the tokens that are to be burned will have to be transferred by the holder in the tx that, ultimately, triggers our Precompile to burn them.

I've checked out the SRF-2: Mint and Burn standard, and here's how I envision this would work:

  1. a token holder $A$ calls the burn() function of the SRF-2 contract, transferring (just) the contract-associated tokens to be burned,
  2. the SRF-2 contract performs the relevant verifications - and calls the SabVM Precompile, transferring it all of the tokens to be burned,
  3. the Precompile does some more checking, ultimately burning the tokens.

In this context,

  1. passing the amount argument to the SRF-2 contract is redundant, as this information can be deduced from the tokens effectively transferred in the current tx;
  2. similarly, the holder argument is redundant, as every holder may only burn the tokens they own themselves;
  3. we need to discuss whether the sub_id argument should, actually, be token_id. An argument for this would be that when a user wants to transfer the token in a tx, they'd need to specify the token_id - and it'd be wrong to expect/ask the users to keep in mind both the sub_id and the token_id of each of the tokens they use.

Another thing worth noting here is that because we won't have total control upon the implementation of the SRF-2 contracts, we won't be able to guarantee that the contracts behave correctly in terms of how much they, actually, burn, etc. However, this is similar to how it's, currently, not the responsibility of Ethereum to validate and approve the logic inside the ERC-20 contract deployed to the blockchain.


Why output both the token_id and the sub_id in the emitted Receipt, though? The token_id is the only thing that should be present there, IMO.

@PaulRBerg PaulRBerg added effort: high Large or difficult task. priority: 1 This is important. It should be dealt with shortly. type: refactor Change that neither fixes a bug nor adds a feature. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise. labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: high Large or difficult task. priority: 1 This is important. It should be dealt with shortly. type: refactor Change that neither fixes a bug nor adds a feature. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.
Projects
None yet
Development

No branches or pull requests

1 participant