Skip to content

Conversation

@ra-phael
Copy link
Collaborator

@ra-phael ra-phael commented Feb 2, 2022

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

  • ERC1238Holdable: The introduction of an additional mapping creates a distinction between the address that owns a badge (often the subject of the claim represented by the badge) and which address currently holds it. A badge owner can then put their badge in escrow while maintaining ownership.
  • ERC1238Stakable: Badge owners can put some of their tokens at stake, effectively giving the rights to a stakeholder to burn them. This can be viewed as a "burning allowance".

@0xpApaSmURf
Copy link

0xpApaSmURf commented Apr 12, 2022

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:

  • Create a I1238Holder.sol:
interface I1238Holder {
    function onBurn(uint256 tokenId) external;
}

that must be implemented by all contracts that want to perform staking.

  • Add hook resolution with events:
//1238Holdable.sol
...
event HeldBurnFailure(address holder, uint256 tokenId, bytes error);

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
Copy link
Collaborator Author

Thank you @0xpApaSmURf! I created a new branch with the preferred option out of the 2 for staking and implemented your suggestion in #10

@ra-phael ra-phael closed this Apr 14, 2022
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.

3 participants