Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Vending machine sketch #364

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions implementation/contracts/deposit/DepositFunding.sol
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,7 @@ library DepositFunding {
_d.setActive();
_d.logFunded();

returnFunderBond(_d);

mintTBTC(_d);
// mint Dposit Owner NFT

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion implementation/contracts/deposit/DepositUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ library DepositUtils {
require(_observedDiff != ValidateSPV.getErrInvalidChain(), "Invalid headers chain");
require(_observedDiff != ValidateSPV.getErrLowWork(), "Insufficient work in a header");

/* TODO: make this better than 6 */
/* TODO: Redo for single proof */
require(
_observedDiff >= _reqDiff.mul(TBTCConstants.getTxProofDifficultyFactor()),
"Insufficient accumulated difficulty in header chain"
Expand Down
94 changes: 94 additions & 0 deletions implementation/contracts/system/Vending.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
pragma solidity ^0.5.10;

import Deposit, TBTCToken, DepositOwnerNFT, DepositBEenficiaryNFT;

contract Vending {

// Check if a Deposit Owner Token (DOT) is qualified
function isQualified(tokenId) public view returns (bool);

// Qualify a DOT
function qualifyDepositOwnerToken(tokenID, proofRequirements) public view returns (bool){
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Shadowfiend, I would like the option to mint a qualified Deposit Owner Token before minting an unqualified 1-conf token. If a user has no need for immediate access The extra TX, time, and cost are all undesirable. I would personally take this option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would file an issue for that. It's a nice-to-have, but seems straightforwardly accomplished with a wrapper contract if we don't have time to add it to the core functionality (simply by calling one function that calls both of the others).

require(tokenId has not been redeemed);
require(within timeout) // expand on timeout in this context
//proof checks out
//proofRequirements <> getQualificationRequirements()
}

// get qualification requirements, currently just number of
// confirmations required. This is 6 + X. X depends on volume
function getQualificationRequirements() public view returns (uint256){
// return current number of blocks needed for qualification
}

// Use a DOT to obtain TBTC (ERC20)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does DOT mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DOT = Deposit Owner Token

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 on this abbreviation. Makes things a bit clearer

// If locked bool == true, maintain exclusive redemption rights,
// keep control of DOT and receive TBTC. The DOT is marked as redeemed. Ownership of the DOT
// now represents the right to redeem the specific custodied UTXO.
// If locked bool == false, Forfeit exclusive redemption rights, Swap DOT
// for Deposit Beneficiary NFT, and receive TBTC.
// DOTs controlled by Vending machine do not need to be marked as redeemed due to
// the TBTC burning requirement enforced by getDepositOwnerNFT()
// this function can be separated into getTBTCLocked() and getTBTCUnlocked()
function GetTBTC(bool locked, uint256 tokenID, proofRequirements ) public {
require(isQualified(tokenId, proofRequirements))
require(within timeout) // expand on timeout in this context
if(!locked){
// swap Deposit Owner NFT for TBTC
// supply Deposit beneficiary NFT
// return
}
// supply TBTC, but no deposit beneficiary NFT
}

// pay required TBTC to retrieve given DOT (in unredeemed state)
function getDepositOwnerNFT(uint256 tokenid) public returns (address) {
require(sended approved sufficient TBTC to be burned);
if(tokenId == 0){
Copy link
Contributor

Choose a reason for hiding this comment

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

For v1 I feel like we can require(tokenId != 0) and force folks to claim specific NFTs. Someone (potentially us) can write a wrapper contract that handles any market making around which token to pull out when. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really like forcing to claim specific NFTs. I think we can create a pretty simple contract to determine redemption queue even without a real market. We can improve on it at a later stage. thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Three platitudes that convey my thoughts: complexity is bad, if it can be externalized it's better, and done is better than perfect 😁

I don't disagree with you that it's doable, but I don't think we lose much by not having this functionality available in a v1. Let's keep the possibility, but make it the last thing we do after the rest of the system is done.

Copy link
Contributor

@liamzebedee liamzebedee Nov 25, 2019

Choose a reason for hiding this comment

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

@Shadowfiend I like the platitudes, just saved them. 😃

I agree with Nich, a redemption queue would be rather simple/elegant. Right now, if two people redeem the same deposit, the higher gas payer will win. Since there's always 1:1 supply of TBTC and unlocked deposits, I'm not sure if a market would help here?

Copy link
Contributor

Choose a reason for hiding this comment

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

To elaborate, I imagine the vending machine might just hold all DOT's that aren't otherwise exclusively owned (ie. forfeited redemption rights). This is a bit more efficient than destroying/recreating DOT's, if we were to do that. And then we could do something like:

tbtc2Dot() {
    if(!depositOwnerTokenId) depositOwnerTokenId = publicDOTs.pop()
    DepositOwnerToken.transferFrom(this, msg.sender, depositOwnerTokenId);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The vending machine should absolutely own any DOTs that have had TBTC minted off of them.

Again, I don't disagree that this is doable---or even desirable! But I'm okay with leaving it aside for now and only adding it if there's time, for the simple reason that it's achievable with an external contract. I think we'll have our hands full already, and this is the kind of thing that will probably take longer to write good tests for than to write in the first place.

Keep in mind that by having deposit owner tokens in the first place, we have the ability to list deposits, filter to open ones, and filter to the ones that are owned by the vending machine. Whether we want to do that in a wrapper contract or in a dApp is another question of interest.

Regardless, I think it's worth holding and starting by requiring that you redeem a specific DOT.

// decide which NFT to return. oldest/random
// some market potential here? (note async here will break redemption wrapper)
// Market can be external system impacting queue order of UTXO redemption.
// could offer an alternative to signer self redemption by paying for redemption priority.
// Paying for DOT queue priority would have to somehow lock the DOT to prevent getDepositOwnerNFT()
// on the specific DOT (since Ownership of the DOT does not guarantee imminent redemption)
// only in redemption Wrapper: getDepositOwnerNFT(RedemptionQueue.Next()) -- not async, cool.
}
// check vending machine for given NFT, revert if it doesn't exist.
// transfer Deposit owner NFT
Copy link
Contributor

Choose a reason for hiding this comment

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

This is “transfer deposit owner NFT to msg.sender”, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it is

// return tokenID
}

// pay required TBTC to retrieve non-specific DOT (in unredeemed state)
function getDepositOwnerNFT() public returns (address) {
getDepositOwnerNFT(0);
}

// redeem a Deposit with TBTC, specific DOT preference.
function redemptionWrapper(uint256 _TokenID) public {
uint256 TokenId;
if(ownerOf(_TokenId) == msg.sender){
tokenId = _TokenID;
}
else{
TokenId = getDepositOwnerNFT(_TokenID);
}
redeemDepositOwnerToken(TokenId)
}

// redeem a Deposit using TBTC. No Specific DOT preference
function redemptionWrapper() public{
uint256 TokenId = getDepositOwnerNFT(0);
redeemDepositOwnerToken(TokenId)
}

// redeem a Dposit using an unredeemed DOT. not TBTC requirement
function redeemDepositOwnerToken(uint256 TokenID) public {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I'm following this one. What does it do exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's meant to be a way to redeem a Deposit using an unredeemed Deposit Owner Token. There should be a redemption flow for the folks opting out of minting fungible TBTC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, of course. Redemption for unlocked tokens would still go through the vending machine because it requires burning TBTC, and we want the vending machine to manage that exclusively.

But there is a TBTC requirement in that case, no?

Copy link
Contributor Author

@NicholasDotSol NicholasDotSol Nov 22, 2019

Choose a reason for hiding this comment

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

I can redeem a Deposit Owner Token for TBTC, but I may chose not to. If I chose not to, the qualified Deposit Owner Token is unredeemed and has a full TBTC worth of value (since it is redeemable for that at any time) so there should be no TBTC requirement here.
Not totally clear on fee handling though. Maybe provide fees with this call

Copy link
Contributor

@Shadowfiend Shadowfiend Nov 25, 2019

Choose a reason for hiding this comment

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

I finally understand some confusion I've had on a couple of your comments :)

The process of turning a DOT into TBTC should not be referred to as redemption. Redemption until now has been turning in TBTC and getting BTC, and for clarity's sake I think we should hold “redemption” to that exclusive meaning. Let's call the process you're referring to here as “redemption” as “minting TBTC off of a Deposit Owner Token”, or “turning a Deposit Owner Token in to the vending machine”. Similarly, if you go TBTC->DOT, let's call this “retrieving a Deposit Owner Token from the vending machine” or something like that.

In this approach, a Deposit Owner Token has no idea whether it's in the vending machine or not, it just changes owners. In fact, I'm a dumbo, and this means we don't need a locked/unlocked states. Deposits should always be locked to their owners. When someone mints TBTC off of a deposit, the deposit's ownership is transferred to the vending machine, allowing it to manage a future redemption transparently.

This leaves us with two flows in the vending machine:

  • Redeeming a deposit owned by the vending machine. This requires 1 TBTC + FEES TBTC.
  • Redeeming a deposit not owned by the vending machine. This requires FEES TBTC.

I can see this being implemented as two public functions, or as one public function with a branch; I think that's up to you. -> I'm finally getting what @NicholasDotSol was going for in his PR + this comment in particular... Following up accordingly.

Copy link
Contributor

@Shadowfiend Shadowfiend Nov 25, 2019

Choose a reason for hiding this comment

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

Of note, the not-owned-by-the-vending-machine redemption flow can be mentally modeled as taking FEES TBTC, minting 1 TBTC (which transfers ownership to the vending machine), then directly entering the “deposit owned by vending machine” redemption path with the resulting 1 TBTC + FEES TBTC. The implementation should probably shortcut around some of this, of course. -> I'm finally getting what @NicholasDotSol was going for in his PR + this comment in particular... Following up accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

And now, to finally understanding the model from the other PR! Because the fee exchange for redemption does not require minting/burning authority, this lands us on an even simpler model, which is what the comment above is describing.

Deposit redemption is managed by the deposit, as today, but it only takes FEES TBTC and distributes it. This is purely a TBTC transfer from the owner of TBTC to the signers.

  • Redeeming a deposit owned by the vending machine requires, as noted in the above-linked comment, exchanging 1 TBTC for the deposit ownership token, then redeeming the underlying deposit (which can be wrapped in a helper later) with FEES TBTC.
  • Redeeming a deposit I own requires a standard redemption, sending FEES TBTC to the deposit.
  • Redeeming a deposit someone else owns requires acquiring ownership from them, or alternatively it requires waiting for liquidation to remove the ownership restriction.

Notably this doesn't require tracking any additional states in the deposit. The only thing we need to do is, when requesting redemption, if the deposit is not in a liquidation state, only the owner is allowed to redeem. If it is in a liquidation state (i.e., a state after courtesy call), anyone may redeem with FEES TBTC.

This brings up the question of how this impacts the liquidation auction for deposits that are not owned by the vending machine ("locked deposits"). I think we need to pick that up next week, and can implement the first parts of this flow while leaving that to a separate consideration.

require(msg.sender == tokenId owner);
require(TokenID is valid and unredeemed)
Deposit(address(tokenId)).requestRedemption() //temp
// msg.sender issues here?
// will need an approval chain for TBTC burn approval requirements
// might need restructure on Deposit to send
// beneficiary reward to correct msg.sender instead of address(this) pass var?
}
}