violetprotocol / ERC1238-token

Implementation of EIP-1238 for non-transferable tokens (fungible & non-fungible).
https://erc1238.notion.site/
16 stars 3 forks source link

Staking options #5

Closed ra-phael closed 2 years ago

ra-phael commented 2 years ago

Introduces 2 options as extensions to enable "staking" of non-transferable tokens:

0xpApaSmURf commented 2 years ago

Revisiting our previous conversations, I think maintaining lifecycle symmetry MUST be an invariant especially in decentralised scenarios where we need to be very careful about trust assumptions. Maintaining lifecycle symmetry alongside trust reduces the risk of fragile systems because otherwise you'll have to build in specific mechanisms to either defend against edge cases or support edge cases.

In this way I would very much be against giving burn rights to a holder of the token.

My suggestion would be what we discussed before as a modification of Holdable:

function _beforeBurn( address holder, address owner, uint256 id, uint256 amount ) internal virtual override { require(_escrowedBalances[holder][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held");

(bool success, bytes result) = holder.call(abi.encodeWithSignature("onBurn(uint256)", id));
if (!success) emit HeldBurnFailure(holder, id, result);

_escrowedBalances[holder][id] -= amount;

}



In this way, we don't disrupt burning flows (as burn logic is always external to any contract that wishes to stake and so it should not obstruct third-party flows), but we still attempt to call out to staking resolution flows for contracts that then have to implement their own way to resolve the case where a staked token is burnt. This leaves the implementation detail to the specific use case where anything could happen on their side and the 1238 token itself is agnostic to that. If it fails we also emit an event to notify them which they should listen to to aid their resolution flows in the case where it fails for some reason.

In the example above I've renamed the variables to something more consistent and clearly identifiable.
ra-phael commented 2 years ago

Thank you @0xpApaSmURf! I created a new branch with the preferred option out of the 2 for staking and implemented your suggestion in https://github.com/violetprotocol/ERC1238-token/pull/10