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 EIP: Fungible Key Bound Token #6808

Merged
merged 26 commits into from
May 2, 2023
Merged

Conversation

KBTStandardAdmin
Copy link
Contributor

A standard interface for Fungible Key Bound Tokens, also known as a Fungible KBT

The Key Bound Token Standard is a new standard focused on self-custodial security. The users of the fungible version can benefit from its integrated decentralized anti-theft mechanism. By binding two additional wallets, this standard prevents attackers and scammers alike to use your asset, even if they gain control of your seed phrase or private key.

@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-erc labels Mar 31, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Mar 31, 2023

✅ All reviewers have approved.

@eth-bot eth-bot changed the title Added a draft EIP for fungible tokens. Add EIP: Fungible Key Bound Token Standard Mar 31, 2023
@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Mar 31, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Mar 31, 2023
@eth-bot eth-bot changed the title Add EIP: Fungible Key Bound Token Standard Add EIP: Fungible Key Bound Token Mar 31, 2023
@github-actions github-actions bot added w-ci Waiting on CI to pass and removed w-ci Waiting on CI to pass labels Mar 31, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Mar 31, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Mar 31, 2023
@eth-bot eth-bot added a-review Waiting on author to review and removed e-review Waiting on editor to review labels Apr 4, 2023
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

Couple broad comments:

  • You'll have to remove all external references, except as permitted in EIP-1: Linking to External Resources. This includes URLs without the scheme (eg. https://), document references like BIP 39, and so on.
  • If possible, can you provide an SVG version of your diagram?
  • Remove build system files (especially package-lock.json)
  • This document has an unusually large number of footnotes. This isn't exactly a problem, but it does break style with other EIPs, and I'd like to see them reduced in number.

EIPS/eip-6808.md Outdated
---
eip: 6808
title: Fungible Key Bound Token
description: An interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately you can't put markdown in the preamble fields.

Suggested change
description: An interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].
description: An interface for Fungible Key Bound Tokens, also known as a Fungible KBT.

EIPS/eip-6808.md Outdated
Comment on lines 14 to 16
A standard interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].

## Abstract
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
A standard interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].
## Abstract
## Abstract
A standard interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].

EIPS/eip-6808.md Outdated

## Abstract

The following standard allows for the implementation of a standard API for tokens within smart contracts and provides basic functionality to the `addBinding`[^2] function. This function creates **Key Wallets**[^3], which are used to `allowTransfer`[^4] or `allowApproval`[^5] in order to conduct a **Safe Transfer**[^6] of fungible tokens. In the process, the tokens are also safely approved so they can be spent by the user or another on-chain third-party entity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't repeat "standard" so much. Perhaps:

The following standardizes an API for tokens [...]

EIPS/eip-6808.md Outdated
requires: 20
---

A standard interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using a footnote for the expanded form of KBT, you can use parentheses:

Suggested change
A standard interface for Fungible Key Bound Tokens, also known as a Fungible **KBT**[^1].
A standard interface for Fungible Key Bound Tokens (FKBT), a subset of of the more general Key Bound Tokens (KBT).

After the first expansion in the document, you're free to use the abbreviation alone.

EIPS/eip-6808.md Outdated
Comment on lines 18 to 20
The following standard allows for the implementation of a standard API for tokens within smart contracts and provides basic functionality to the `addBinding`[^2] function. This function creates **Key Wallets**[^3], which are used to `allowTransfer`[^4] or `allowApproval`[^5] in order to conduct a **Safe Transfer**[^6] of fungible tokens. In the process, the tokens are also safely approved so they can be spent by the user or another on-chain third-party entity.

This functionality is fully optional and security features malleable to suit one's needs. To activate it, the holder must add 2 different wallets when emitting the `addBinding`[^2] function. These will be known as `_keyWallet1`[^7] and `_keyWallet2`[^8]. If the user does not activate the assets security feature, **KBTs**[^1] **Default Behavior**[^22] is the same as a traditional fungible [_ERC-20_](../EIPS/eip-20.md) token. However, even when the security feature is activated, the standard has **Default Values**[^23] which the user can input to achieve the same result.
Copy link
Contributor

Choose a reason for hiding this comment

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

Your abstract, so far, goes too deep into the technical details without giving a high enough level overview of the topic. What is a Key Bound Token? You don't really define the concept of "allow" (as in allowTransfer and allowApproval.)

EIPS/eip-6808.md Outdated
Comment on lines 435 to 550
) internal view returns (bool) {
TransferConditions memory conditions = _transferConditions[_account];

if (conditions.allFunds) {
return true;
}

if (
(conditions.amount == 0 &&
conditions.time == 0 &&
conditions.to == address(0) &&
!conditions.allFunds) ||
(conditions.amount > 0 && conditions.amount < _amount) ||
(conditions.time > 0 && conditions.time < block.timestamp) ||
(conditions.to != address(0) && conditions.to != _to)
) {
return false;
}

return true;
}
```

#### `_getAccountHolder` function

This function identifies and returns the `_holder` account starting from the `sender` address.

If the `sender` is not a `keyWallet` zero address is returned (`0x0`).

```solidity
function _getAccountHolder() internal view returns (address) {
address sender = _msgSender();
return
_firstAccounts[sender].accountHolderWallet != address(0)
? _firstAccounts[sender].accountHolderWallet
: (
_secondAccounts[sender].accountHolderWallet != address(0)
? _secondAccounts[sender].accountHolderWallet
: address(0)
);
}
```

#### `_getOtherSecureWallet` function

This function identifies and returns the other `keyWallet` starting from the `sender` address.

If the `sender` is not a `keyWallet` zero address is returned (`0x0`).

```solidity
function _getOtherSecureWallet() internal view returns (address) {
address sender = _msgSender();
address accountHolder = _getAccountHolder();

return
_holderAccounts[accountHolder].firstWallet == sender
? _holderAccounts[accountHolder].secondWallet
: _holderAccounts[accountHolder].firstWallet;
}
```

#### `_isApprovalAllowed` function

This function returns `true` if the `_time`, set in `allowApproval` function, has not elapsed yet.

```solidity
function _isApprovalAllowed(address account) internal view returns (bool) {
return _approvalConditions[account].time >= block.timestamp;
}
```

#### `_afterTokenTransfer` function

This function is called after a successful `transfer`, `transferFrom`, `mint` and `burn`.

This function fires `Egress` event when loosing a holder and `Ingress` event when gaining a holder.

```solidity
function _afterTokenTransfer(
address from,
address to,
uint256 amount
) internal virtual {
// region update secureAccounts
if (isSecureWallet(from) && balanceOf(from) == 0) {
delete _firstAccounts[_holderAccounts[from].firstWallet];
delete _secondAccounts[_holderAccounts[from].secondWallet];
delete _holderAccounts[from];
}
// endregion

if (balanceOf(from) == 0) {
emit Egress(from, amount);
}

if (balanceOf(to) == amount) {
emit Ingress(to, amount);
}
}
```
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
### **Internal functions**
#### `_hasAllowedTransfer` function
The function returns `true` if:
- the `_account` has allowed for transfer all funds through `_allFunds` parameter
- or the `_account` has allowed for transfer an `_amount` of funds, for `_to` address and if the `time` has not elapsed
In all other cases the function will return `false`.
```solidity
function _hasAllowedTransfer(
address _account,
uint256 _amount,
address _to
) internal view returns (bool) {
TransferConditions memory conditions = _transferConditions[_account];
if (conditions.allFunds) {
return true;
}
if (
(conditions.amount == 0 &&
conditions.time == 0 &&
conditions.to == address(0) &&
!conditions.allFunds) ||
(conditions.amount > 0 && conditions.amount < _amount) ||
(conditions.time > 0 && conditions.time < block.timestamp) ||
(conditions.to != address(0) && conditions.to != _to)
) {
return false;
}
return true;
}
```
#### `_getAccountHolder` function
This function identifies and returns the `_holder` account starting from the `sender` address.
If the `sender` is not a `keyWallet` zero address is returned (`0x0`).
```solidity
function _getAccountHolder() internal view returns (address) {
address sender = _msgSender();
return
_firstAccounts[sender].accountHolderWallet != address(0)
? _firstAccounts[sender].accountHolderWallet
: (
_secondAccounts[sender].accountHolderWallet != address(0)
? _secondAccounts[sender].accountHolderWallet
: address(0)
);
}
```
#### `_getOtherSecureWallet` function
This function identifies and returns the other `keyWallet` starting from the `sender` address.
If the `sender` is not a `keyWallet` zero address is returned (`0x0`).
```solidity
function _getOtherSecureWallet() internal view returns (address) {
address sender = _msgSender();
address accountHolder = _getAccountHolder();
return
_holderAccounts[accountHolder].firstWallet == sender
? _holderAccounts[accountHolder].secondWallet
: _holderAccounts[accountHolder].firstWallet;
}
```
#### `_isApprovalAllowed` function
This function returns `true` if the `_time`, set in `allowApproval` function, has not elapsed yet.
```solidity
function _isApprovalAllowed(address account) internal view returns (bool) {
return _approvalConditions[account].time >= block.timestamp;
}
```
#### `_afterTokenTransfer` function
This function is called after a successful `transfer`, `transferFrom`, `mint` and `burn`.
This function fires `Egress` event when loosing a holder and `Ingress` event when gaining a holder.
```solidity
function _afterTokenTransfer(
address from,
address to,
uint256 amount
) internal virtual {
// region update secureAccounts
if (isSecureWallet(from) && balanceOf(from) == 0) {
delete _firstAccounts[_holderAccounts[from].firstWallet];
delete _secondAccounts[_holderAccounts[from].secondWallet];
delete _holderAccounts[from];
}
// endregion
if (balanceOf(from) == 0) {
emit Egress(from, amount);
}
if (balanceOf(to) == amount) {
emit Ingress(to, amount);
}
}
```

Generally you should only standardize functions that are externally visible in the interface itself. The standard should provide enough information that multiple implementations can interoperate correctly, without specifying how the implementations should work. These kind of choices should be left up to the implementers.

EIPS/eip-6808.md Outdated
[^6]: A **Safe Transfer** is when 1 of the **Key Wallets** safely approved the use of the fungible token.
[^7]: The `keyWallet1` is 1 of the 2 **Key Wallets** set when calling the `addBinding` function.
[^8]: The `keyWallet2` is 1 of the 2 **Key Wallets** set when calling the `addBinding` function.
[^9]: The [_ERC-20_](../EIPS/eip-20.md) is the token standard for creating smart contract-enabled fungible tokens to be used in the Ethereum ecosystem. Source - investopedia.com/news/what-erc20-and-what-does-it-mean-ethereum/#:~:text=What's%20the%20Difference%20Be website.
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
[^9]: The [_ERC-20_](../EIPS/eip-20.md) is the token standard for creating smart contract-enabled fungible tokens to be used in the Ethereum ecosystem. Source - investopedia.com/news/what-erc20-and-what-does-it-mean-ethereum/#:~:text=What's%20the%20Difference%20Be website.
[^9]: The [ERC-20](./eip-20.md) is the token standard for creating smart contract-enabled fungible tokens to be used in the Ethereum ecosystem

EIPS/eip-6808.md Outdated
[^7]: The `keyWallet1` is 1 of the 2 **Key Wallets** set when calling the `addBinding` function.
[^8]: The `keyWallet2` is 1 of the 2 **Key Wallets** set when calling the `addBinding` function.
[^9]: The [_ERC-20_](../EIPS/eip-20.md) is the token standard for creating smart contract-enabled fungible tokens to be used in the Ethereum ecosystem. Source - investopedia.com/news/what-erc20-and-what-does-it-mean-ethereum/#:~:text=What's%20the%20Difference%20Be website.
[^10]: Security known as _BIP-39_, defines how wallets create seed phrases and generate encryption keys. Source - vault12.com/securemycrypto/crypto-security-basics/what-is-bip39/ website.
Copy link
Contributor

Choose a reason for hiding this comment

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

External links, like to BIP-39 or to vault12 are not permitted.

Suggested change
[^10]: Security known as _BIP-39_, defines how wallets create seed phrases and generate encryption keys. Source - vault12.com/securemycrypto/crypto-security-basics/what-is-bip39/ website.

EIPS/eip-6808.md Outdated
[^14]: The `_amount` represents the amount of the asset intended to be spent.
[^15]: The `_time` in `allowTransfer` represents the number of blocks a `transfer` can take place in.
[^16]: The `_address` represents tha address that the asset will be sent too.
[^17]: The denomination of the cryptocurrency ether (ETH), used on the Ethereum network to buy and sell goods and services is known as _GWEI_. Source - investopedia.com/terms/g/gwei-ethereum.asp#:~:text=Key%20Takeaways-,Gwei%20is%20a%20denomination%20of%20the%20cryptocurrency%20ether%20(ETH)%2C,to%20specify%20Ethereum%20gas%20prices website.
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
[^17]: The denomination of the cryptocurrency ether (ETH), used on the Ethereum network to buy and sell goods and services is known as _GWEI_. Source - investopedia.com/terms/g/gwei-ethereum.asp#:~:text=Key%20Takeaways-,Gwei%20is%20a%20denomination%20of%20the%20cryptocurrency%20ether%20(ETH)%2C,to%20specify%20Ethereum%20gas%20prices website.

gwei is a well known term to the audience of this repository.

@@ -0,0 +1,391 @@
// SPDX-License-Identifier: CCO
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
// SPDX-License-Identifier: CCO
// SPDX-License-Identifier: CC0-1.0

KBTStandardAdmin and others added 4 commits April 8, 2023 09:00
* Updated the Specification section and replaced the png with and svg file

* Fixed the assets issue with package-lock

* Fixed the licence in the KBT contract

* Description + Abstract + Motivation

Description bullet points and bolding options

* description bolding fix

* DApp = dApp funds = FKBT security considerations

* Footnotes Updated

* Rationale Removal

* rationale change

* footnote rationale

* Funcationality Correction

* Backwards Compatibility Section

* Footnotes Updated

* Applying markdown preferred style

---------

Co-authored-by: KBT Admin <[email protected]>
Co-authored-by: NickZCZ <[email protected]>
Co-authored-by: MihaiORO <[email protected]>
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Apr 18, 2023
EIPS/eip-6808.md Outdated

The following standardizes an API for tokens within smart contracts and provides basic functionality to the `addBinding`[^1] function. This function designates **Key Wallets**[^2], which are responsible for conducting a **Safe Transfer**[^5]. During this process, **FKBTs** are safely approved so they can be spent by the user or an on-chain third-party entity.

The premise of **FKBTs** is to provide fully optional security features built directly into the fungible asset, via the concept of _allow_ found in the `allowTransfer`[^3] and `allowApproval`[^4] functions. These functions are called by one of the **Key Wallets**[^2] and _allow_ the **Holding Wallet**[^9] to either call the already familier `transfer` and `approve` function found in the [_ERC-20_](./eip-20.md). Responsibility for the **FKBT** is therefore split. The **Holding Wallet**[^9] contains the asset and **Key Wallets**[^2] have authority over how the assets can be spent or approved. **Default Behaviors**[^17] of a traditional fungible _ERC-20_ can be achieved by simply never using the `addBinding`[^1] function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny style complaints: don't italicize ERC/EIP references, and don't use "the" before them. You can use backticks (`) for code references (like interface names) though.

Small spelling correction as well.

Suggested change
The premise of **FKBTs** is to provide fully optional security features built directly into the fungible asset, via the concept of _allow_ found in the `allowTransfer`[^3] and `allowApproval`[^4] functions. These functions are called by one of the **Key Wallets**[^2] and _allow_ the **Holding Wallet**[^9] to either call the already familier `transfer` and `approve` function found in the [_ERC-20_](./eip-20.md). Responsibility for the **FKBT** is therefore split. The **Holding Wallet**[^9] contains the asset and **Key Wallets**[^2] have authority over how the assets can be spent or approved. **Default Behaviors**[^17] of a traditional fungible _ERC-20_ can be achieved by simply never using the `addBinding`[^1] function.
The premise of **FKBTs** is to provide fully optional security features built directly into the fungible asset, via the concept of _allow_ found in the `allowTransfer`[^3] and `allowApproval`[^4] functions. These functions are called by one of the **Key Wallets**[^2] and _allow_ the **Holding Wallet**[^9] to either call the already familiar `transfer` and `approve` function found in [ERC-20](./eip-20.md). Responsibility for the **FKBT** is therefore split. The **Holding Wallet**[^9] contains the asset and **Key Wallets**[^2] have authority over how the assets can be spent or approved. **Default Behaviors**[^17] of a traditional fungible ERC-20 can be achieved by simply never using the `addBinding`[^1] function.

EIPS/eip-6808.md Outdated

## Specification

### ERC-N (Token Contract)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this heading a placeholder?

EIPS/eip-6808.md Outdated

A standard interface for Fungible Key Bound Tokens (**FKBT/s**), a subset of the more general Key Bound Tokens (**KBT/s**).

The following standardizes an API for tokens within smart contracts and provides basic functionality to the `addBinding`[^1] function. This function designates **Key Wallets**[^2], which are responsible for conducting a **Safe Transfer**[^5]. During this process, **FKBTs** are safely approved so they can be spent by the user or an on-chain third-party entity.
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
The following standardizes an API for tokens within smart contracts and provides basic functionality to the `addBinding`[^1] function. This function designates **Key Wallets**[^2], which are responsible for conducting a **Safe Transfer**[^5]. During this process, **FKBTs** are safely approved so they can be spent by the user or an on-chain third-party entity.
The following standardizes an API for tokens within smart contracts and provides basic functionality to the `addBindings`[^1] function. This function designates **Key Wallets**[^2], which are responsible for conducting a **Safe Transfer**[^5]. During this process, **FKBTs** are safely approved so they can be spent by the user or an on-chain third-party entity.

EIPS/eip-6808.md Outdated

## Reference Implementation

The GitHub repository [KBTstandard repository](../assets/eip-6808/README.md) contains the implementation. There's also a [diagram](../assets/eip-6808/Contract%20Interactions%20diagram.svg) with the contract interactions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this lives in the assets directory now, it's odd to mention the "KBTstandard repository".

* Rephrased the Reference Implementation.

* Changed the heading in Specifications section

* Linked the functions to the headings instead of the footnotes + removed unused footnotes

* Security Considerations redirect - rationale section

* Spelling Check #1 + BIP / SLIP removal + the ERC check + Footnote removal & # Update

* motivation topic bolding change

* Motivation FTC sentence

* FKBT footnotes and corrections update

* Key Wallet Reduction

---------

Co-authored-by: KBT Admin <[email protected]>
Co-authored-by: NickZCZ <[email protected]>
Co-authored-by: MihaiORO <[email protected]>
@github-actions
Copy link

The commit 8f7bd23 (as a parent of efbbc8a) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Apr 25, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Apr 25, 2023
NarcisCRO
NarcisCRO previously approved these changes Apr 25, 2023
Copy link
Contributor

@NarcisCRO NarcisCRO left a comment

Choose a reason for hiding this comment

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

Hi @SamWilsn
We fixed the last comments and we're ready for merge.

EIPS/eip-6808.md Outdated Show resolved Hide resolved
EIPS/eip-6808.md Outdated Show resolved Hide resolved
EIPS/eip-6808.md Outdated Show resolved Hide resolved
EIPS/eip-6808.md Outdated Show resolved Hide resolved
assets/eip-6808/README.md Outdated Show resolved Hide resolved
@eth-bot eth-bot enabled auto-merge (squash) May 2, 2023 18:35
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 82db98a into ethereum:master May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-review Waiting on author to review c-new Creates a brand new proposal e-consensus Waiting on editor consensus s-draft This EIP is a Draft t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants