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

Remove TBTC minting from Vending Machine Deposit qualification #377

Merged
merged 14 commits into from
Dec 6, 2019
Merged

Remove TBTC minting from Vending Machine Deposit qualification #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
55eb406
Remove ACL comment
Dec 3, 2019
1c47b9f
Update proof function and TBTC minting
Dec 3, 2019
0886d7e
Update tests for new wrapper and TBTC value
Dec 3, 2019
b039d70
Reorganize tests to match dependencies
Dec 3, 2019
f5f7bd3
Remove .only from VendingMachine tests
Dec 3, 2019
6148eac
Remove old comment
Dec 3, 2019
d98d4a0
Merge branch 'master' into remove-proof-minting
Dec 5, 2019
899cfab
mint signer fee on dotToTbtc exchange
Dec 6, 2019
8171ba8
qualifyDepositTbtcWrapper -> unqualifiedDepositToTbtc
Dec 6, 2019
1b08301
Group funcinality and separate context
Dec 6, 2019
7e0602c
Update tests
Dec 6, 2019
da8d414
Always burn full lotSize TBTC for DOT
Dec 6, 2019
6e14515
Update tests
Dec 6, 2019
fb589b3
Merge branch 'master' into remove-proof-minting
Dec 6, 2019
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
2 changes: 1 addition & 1 deletion implementation/contracts/deposit/DepositFunding.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ library DepositFunding {
bytes memory _merkleProof,
uint256 _txIndexInBlock,
bytes memory _bitcoinHeaders
) public returns (bool) { //TODO Implement ACL for Vending Machine
) public returns (bool) {

require(_d.inAwaitingBTCFundingProof(), "Not awaiting funding");

Expand Down
90 changes: 56 additions & 34 deletions implementation/contracts/system/VendingMachine.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,6 @@ contract VendingMachine {
depositOwnerToken = DepositOwnerToken(_depositOwnerToken);
}

/// @notice Qualifies a deposit for minting TBTC.
function qualifyDeposit(
address payable _depositAddress,
bytes4 _txVersion,
bytes memory _txInputVector,
bytes memory _txOutputVector,
bytes4 _txLocktime,
uint8 _fundingOutputIndex,
bytes memory _merkleProof,
uint256 _txIndexInBlock,
bytes memory _bitcoinHeaders
) public {
Deposit _d = Deposit(_depositAddress);
require(
_d.provideBTCFundingProof(
_txVersion,
_txInputVector,
_txOutputVector,
_txLocktime,
_fundingOutputIndex,
_merkleProof,
_txIndexInBlock,
_bitcoinHeaders
),
"failed to provide funding proof");
// mint the signer fee to the Deposit
tbtcToken.mint(_depositAddress, DepositUtils.signerFee());
}

/// @notice Determines whether a deposit is qualified for minting TBTC.
/// @param _depositAddress the address of the deposit
function isQualified(address payable _depositAddress) public returns (bool) {
Expand All @@ -63,8 +34,9 @@ contract VendingMachine {
require(depositOwnerToken.exists(_dotId), "Deposit Owner Token does not exist");
require(isQualified(address(_dotId)), "Deposit must be qualified");

require(tbtcToken.balanceOf(msg.sender) >= getDepositValueLessSignerFee(), "Not enough TBTC for DOT exchange");
tbtcToken.burnFrom(msg.sender, getDepositValueLessSignerFee());
uint256 getDepositValue = getDepositValue();
Copy link
Contributor

Choose a reason for hiding this comment

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

@NicholasDotSol can you fix up getDepositValue in #385?

require(tbtcToken.balanceOf(msg.sender) >= getDepositValue, "Not enough TBTC for DOT exchange");
tbtcToken.burnFrom(msg.sender, getDepositValue);

// TODO do we need the owner check below? transferFrom can be approved for a user, which might be an interesting use case.
require(depositOwnerToken.ownerOf(_dotId) == address(this), "Deposit is locked");
Expand All @@ -79,16 +51,66 @@ contract VendingMachine {
require(isQualified(address(_dotId)), "Deposit must be qualified");

depositOwnerToken.transferFrom(msg.sender, address(this), _dotId);
tbtcToken.mint(msg.sender, getDepositValueLessSignerFee());

// If the backing Deposit does not have a signer fee in escrow, mint it.
if(tbtcToken.balanceOf(address(_dotId)) < DepositUtils.signerFee()){
tbtcToken.mint(msg.sender, getDepositValueLessSignerFee());
tbtcToken.mint(address(_dotId), DepositUtils.signerFee());
Copy link
Contributor

@Shadowfiend Shadowfiend Dec 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This still feels wrong---in particular, that we're potentially at nonzero escrow but are minting the full signer fee to the deposit, putting us at > signer fee escrow. I can't reason my way all the way through it right now, but I want us to work through it a little.

For now, let's roll with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't find any issues with it.

Escrow gets distributed as a final step before contract halt. Any TBTC more than signerFee left in the Deposit is just locked, and this will be user error (like sending to address(0)). There is no logical reason for more TBTC to be in the deposit. If someone wants to do that for some reason there's no sence in preventing them, they're not gaining anything so shrug.

Will take a deeper look at the state machine to make sure there are no corner cases

}
else{
tbtcToken.mint(msg.sender, getDepositValue());
}
}

// WRAPPERS

/// @notice Qualifies a deposit and mints TBTC.
/// @dev User must allow VendingManchine to transfer DOT
function unqualifiedDepositToTbtc(
address payable _depositAddress,
bytes4 _txVersion,
bytes memory _txInputVector,
bytes memory _txOutputVector,
bytes4 _txLocktime,
uint8 _fundingOutputIndex,
bytes memory _merkleProof,
uint256 _txIndexInBlock,
bytes memory _bitcoinHeaders
) public {
Deposit _d = Deposit(_depositAddress);
require(
_d.provideBTCFundingProof(
_txVersion,
_txInputVector,
_txOutputVector,
_txLocktime,
_fundingOutputIndex,
_merkleProof,
_txIndexInBlock,
_bitcoinHeaders
),
"failed to provide funding proof");

dotToTbtc(uint256(_depositAddress));
}

// HELPERS

// TODO temporary helper function
/// @notice Gets the Deposit lot size less signer fees
/// @return amount in TBTC
function getDepositValueLessSignerFee() internal returns (uint) {
function getDepositValue() internal returns (uint) {
uint256 _multiplier = TBTCConstants.getSatoshiMultiplier();
uint256 _signerFee = DepositUtils.signerFee();
uint256 _totalValue = TBTCConstants.getLotSize().mul(_multiplier);
return _totalValue;
}

// TODO temporary helper function
/// @notice Gets the Deposit lot size
/// @return amount in TBTC
function getDepositValueLessSignerFee() internal returns (uint) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we wouldn't just… Do this inline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, think this looks cleaner though. Expecting to use one of these functions in redemption wrappers

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm… Whether or not we escrow signing fees, we're unlikely to be needing the lot size minus signer fee except at mint-time. Everything else is done in terms of lot size. But we can wait and see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we're unlikely to be needing the lot size minus signer fee except at mint-time

agree

But we can wait and see.

👍

uint256 _signerFee = DepositUtils.signerFee();
uint256 _totalValue = getDepositValue();
return _totalValue.sub(_signerFee);
}
}
132 changes: 74 additions & 58 deletions implementation/test/VendingMachineTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ contract('VendingMachine', (accounts) => {
let assertBalance
let dotId

// For lack of a better design, this is the amount of TBTC exchanged for DOT's.
const depositValueLessSignerFee = '995000000000000000'
// this is the amount of TBTC exchanged for a DOT.
let depositValue
let signerFee

before(async () => {
// VendingMachine relies on linked libraries, hence we use deploySystem for consistency.
Expand All @@ -95,57 +96,11 @@ contract('VendingMachine', (accounts) => {

tbtcSystemStub.forceMint(accounts[4], web3.utils.toBN(deployed.TestDeposit.address))
dotId = await web3.utils.toBN(testInstance.address)
})

describe('#qualifyDeposit', async () => {
before(async () => {
await tbtcSystemStub.setCurrentDiff(currentDifficulty)
await testInstance.setState(utils.states.AWAITING_BTC_FUNDING_PROOF)
await testInstance.setSigningGroupPublicKey(_signerPubkeyX, _signerPubkeyY)
})

beforeEach(async () => {
await createSnapshot()
})

afterEach(async () => {
await restoreSnapshot()
})

it('qualifies a Deposit', async () => {
const blockNumber = await web3.eth.getBlock('latest').number

await vendingMachine.qualifyDeposit(testInstance.address, _version, _txInputVector, _txOutputVector, _txLocktime, _fundingOutputIndex, _merkleProof, _txIndexInBlock, _bitcoinHeaders)

const UTXOInfo = await testInstance.getUTXOInfo.call()
assert.equal(UTXOInfo[0], _outValueBytes)
assert.equal(UTXOInfo[2], _expectedUTXOoutpoint)

const signingGroupRequestedAt = await testInstance.getSigningGroupRequestedAt.call()
assert(signingGroupRequestedAt.eqn(0), 'signingGroupRequestedAt not updated')

const fundingProofTimerStart = await testInstance.getFundingProofTimerStart.call()
assert(fundingProofTimerStart.eqn(0), 'fundingProofTimerStart not updated')

const depositState = await testInstance.getState.call()
expect(depositState).to.eq.BN(utils.states.ACTIVE)

const eventList = await tbtcSystemStub.getPastEvents('Funded', { fromBlock: blockNumber, toBlock: 'latest' })
assert.equal(eventList.length, 1)
})

it('mints signer fee to the deposit', async () => {
const initialBalance = await tbtcToken.balanceOf(testInstance.address)

await vendingMachine.qualifyDeposit(testInstance.address, _version, _txInputVector, _txOutputVector, _txLocktime, _fundingOutputIndex, _merkleProof, _txIndexInBlock, _bitcoinHeaders)

const signerFee = await deployed.TestDepositUtils.signerFee()

const finalBalance = await tbtcToken.balanceOf(testInstance.address)
const expectedBalance = new BN(initialBalance).add(new BN(signerFee))

expect(finalBalance).to.eq.BN(expectedBalance)
})
const lotSize = await deployed.TBTCConstants.getLotSize()
const satoshiMultiplier = await deployed.TBTCConstants.getSatoshiMultiplier()
signerFee = await deployed.TestDepositUtils.signerFee.call()
depositValue = lotSize.mul(satoshiMultiplier)
})

describe('#isQualified', async () => {
Expand Down Expand Up @@ -181,7 +136,18 @@ contract('VendingMachine', (accounts) => {

await vendingMachine.dotToTbtc(dotId)

await assertBalance.tbtc(accounts[0], depositValueLessSignerFee)
await assertBalance.tbtc(accounts[0], depositValue.sub(signerFee))
})

it('mints full lot size if backing deposit has signer fee escrowed', async () => {
await tbtcToken.forceMint(testInstance.address, signerFee)

await depositOwnerToken.forceMint(accounts[0], dotId)
await depositOwnerToken.approve(vendingMachine.address, dotId, { from: accounts[0] })

await vendingMachine.dotToTbtc(dotId)

await assertBalance.tbtc(accounts[0], depositValue)
})

it('fails if deposit not qualified', async () => {
Expand Down Expand Up @@ -226,8 +192,8 @@ contract('VendingMachine', (accounts) => {

it('converts TBTC to DOT', async () => {
await depositOwnerToken.forceMint(vendingMachine.address, dotId)
await tbtcToken.forceMint(accounts[0], depositValueLessSignerFee)
await tbtcToken.approve(vendingMachine.address, depositValueLessSignerFee, { from: accounts[0] })
await tbtcToken.forceMint(accounts[0], depositValue)
await tbtcToken.approve(vendingMachine.address, depositValue, { from: accounts[0] })

const fromBlock = await web3.eth.getBlockNumber()
await vendingMachine.tbtcToDot(dotId)
Expand All @@ -236,7 +202,7 @@ contract('VendingMachine', (accounts) => {
const tbtcBurntEvent = events[0]
expect(tbtcBurntEvent.returnValues.from).to.equal(accounts[0])
expect(tbtcBurntEvent.returnValues.to).to.equal(utils.address0)
expect(tbtcBurntEvent.returnValues.value).to.equal(depositValueLessSignerFee)
expect(tbtcBurntEvent.returnValues.value).to.equal(depositValue.toString())

expect(
await depositOwnerToken.ownerOf(dotId)
Expand Down Expand Up @@ -273,13 +239,63 @@ contract('VendingMachine', (accounts) => {
// Deposit is locked if the Deposit Owner Token is not owned by the vending machine
const depositOwner = accounts[1]
await depositOwnerToken.forceMint(depositOwner, dotId)
await tbtcToken.forceMint(accounts[0], depositValueLessSignerFee)
await tbtcToken.approve(vendingMachine.address, depositValueLessSignerFee, { from: accounts[0] })
await tbtcToken.forceMint(accounts[0], depositValue)
await tbtcToken.approve(vendingMachine.address, depositValue, { from: accounts[0] })

await expectThrow(
vendingMachine.tbtcToDot(dotId),
'Deposit is locked.'
)
})
})

describe('#unqualifiedDepositToTbtc', async () => {
before(async () => {
await tbtcSystemStub.setCurrentDiff(currentDifficulty)
await testInstance.setState(utils.states.AWAITING_BTC_FUNDING_PROOF)
await testInstance.setSigningGroupPublicKey(_signerPubkeyX, _signerPubkeyY)
})

beforeEach(async () => {
await createSnapshot()
})

afterEach(async () => {
await restoreSnapshot()
})

it('qualifies a Deposit', async () => {
await depositOwnerToken.forceMint(accounts[0], dotId)
await depositOwnerToken.approve(vendingMachine.address, dotId, { from: accounts[0] })
const blockNumber = await web3.eth.getBlock('latest').number

await vendingMachine.unqualifiedDepositToTbtc(testInstance.address, _version, _txInputVector, _txOutputVector, _txLocktime, _fundingOutputIndex, _merkleProof, _txIndexInBlock, _bitcoinHeaders)

const UTXOInfo = await testInstance.getUTXOInfo.call()
assert.equal(UTXOInfo[0], _outValueBytes)
assert.equal(UTXOInfo[2], _expectedUTXOoutpoint)

const signingGroupRequestedAt = await testInstance.getSigningGroupRequestedAt.call()
assert(signingGroupRequestedAt.eqn(0), 'signingGroupRequestedAt not updated')

const fundingProofTimerStart = await testInstance.getFundingProofTimerStart.call()
assert(fundingProofTimerStart.eqn(0), 'fundingProofTimerStart not updated')

const depositState = await testInstance.getState.call()
expect(depositState).to.eq.BN(utils.states.ACTIVE)

const eventList = await tbtcSystemStub.getPastEvents('Funded', { fromBlock: blockNumber, toBlock: 'latest' })
assert.equal(eventList.length, 1)
})

it('mints TBTC to the DOT owner and siger fee to Deposit', async () => {
await depositOwnerToken.forceMint(accounts[0], dotId)
await depositOwnerToken.approve(vendingMachine.address, dotId, { from: accounts[0] })

await vendingMachine.unqualifiedDepositToTbtc(testInstance.address, _version, _txInputVector, _txOutputVector, _txLocktime, _fundingOutputIndex, _merkleProof, _txIndexInBlock, _bitcoinHeaders)

await assertBalance.tbtc(accounts[0], depositValue.sub(signerFee))
await assertBalance.tbtc(testInstance.address, signerFee)
})
})
})