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

Add ERC: Soul Resonance Token #414

Closed
wants to merge 2 commits into from
Closed

Conversation

mike0x1024
Copy link

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented May 7, 2024

File ERCS/erc-7699.md

Requires 1 more reviewers from @axic, @g11tech, @SamWilsn, @xinbenlv

@github-actions github-actions bot added the w-ci label May 7, 2024
@eip-review-bot eip-review-bot changed the title Add ERC: Soul Resonance Token Add ERC: Soul Resonance Token May 7, 2024
@@ -0,0 +1,90 @@
---
eip: 7699
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eip: 7699
eip: 7704

Assigning next sequential EIP/ERC/RIP number.

Please also update the filename.


## Abstract

This EIP proposes the following improvements to Soul Bond Tokens:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This EIP proposes the following improvements to Soul Bond Tokens:
This EIP proposes the following improvements to Soul Bound Tokens:

Assume you mean bound?

title: Soul Resonance Token
description: this EIP defines an interface for Soul Resonance Tokens
author: Mike Smith (@mike0x1024), Abe Johnson <abe@burve.io>, Alvis Kaiser <alvis@burve.io>, WurDst Tsao <wurst@burve.io>
discussions-to: https://ethereum-magicians.org/t/1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a topic in Ethereum Magicians with a link to this PR.

---
eip: 7699
title: Soul Resonance Token
description: this EIP defines an interface for Soul Resonance Tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your description just repeats your title. Try to expand on the idea you introduce there, instead of just restating it. Perhaps explain what a Soul Resonance Token is?

Comment on lines +16 to +18
This EIP proposes the following improvements to Soul Bond Tokens:
Extensibility to [ERC-20](./erc-20.md).
Token Minting and Burning while preserving non-transferability.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your abstract should layout the goal of the proposal, and give the reader a high level technical overview of how it accomplishes that goal. Yours is probably a bit light on the technical side.

Comment on lines +32 to +33
Notes:
The contract is designed to be compiled with a Solidity compiler version greater than or equal to 0.8.20 but less than 0.9.0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally standards should be applicable across all Solidity versions. You shouldn't need to note this.

Suggested change
Notes:
The contract is designed to be compiled with a Solidity compiler version greater than or equal to 0.8.20 but less than 0.9.0.

Notes:
The contract is designed to be compiled with a Solidity compiler version greater than or equal to 0.8.20 but less than 0.9.0.

The smart contract realising this EIP mandate must fully incorporate all methods prescribed by [ERC-20](./erc-20.md), with the exception of transfer operations. Furthermore, exclusivity to implementing solely this EIP is required.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is vaguely worded. For example, if an implementer wants to support ERC-165, would this prevent them?

### Internal _update Function Override

```solidity
function _update(address from, address to, uint256 value) internal virtual override {
Copy link
Collaborator

Choose a reason for hiding this comment

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

_update is not part of the standard ERC-20. Are you thinking of OpenZeppelin perhaps?

EIPs should only specify the externally visible behaviours of contracts, and not their implementation specifics. _update looks like an internal function, and so should not be part of the standard.


```solidity
function mint(address to, uint256 amount) public {
_mint(to, amount);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment here: _mint looks like an internal function.


```solidity
function burn(address from, uint256 amount) public {
_burn(from, amount);
Copy link
Collaborator

Choose a reason for hiding this comment

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

And again here with _burn.

Comment on lines +77 to +80
## Citation

Please cite this document as:
Burve Labs. "Bridging the Gap: Soul Resonance Token (SRT) Non-transferable yet Tradable Asset Type based on Bonding Curve"Burve Labs. 20 Apr. 2024.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## Citation
Please cite this document as:
Burve Labs. "Bridging the Gap: Soul Resonance Token (SRT) Non-transferable yet Tradable Asset Type based on Bonding Curve"Burve Labs. 20 Apr. 2024.

The renderer will generate this section for you.

Comment on lines +82 to +87
## References

**Implementations**

- [ERC-7699 Reference Implementation](https://github.com/mike0x1024/srt-template)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a non-standard section of EIPs. If you'd like to include a reference implementation, you may do so in the assets directory and link to it there.

Copy link

The commit 6a7edd3 (as a parent of f50f283) contains errors.
Please inspect the Run Summary for details.

@mike0x1024 mike0x1024 closed this by deleting the head repository Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants