PoC discount on fees when tokens are staked#13
Conversation
migrations/6_initialize_721token.js
Outdated
| case 'rinkeby': | ||
| erc20TokenAddress = '0xb05e292f89c6a82f5ed1be694dc7b6444866b364' | ||
| initialFees = 10 | ||
| initialFees = 10 ** 18 // 1 token |
There was a problem hiding this comment.
This may need to be a BigNumber, otherwise it'll loose some precision.
| const CodexTitleProxy = artifacts.require('./CodexTitleProxy.sol') | ||
|
|
||
| module.exports = async (deployer, network, accounts) => { | ||
| const proxiedCodexTitle = CodexTitle.at(CodexTitleProxy.address) |
There was a problem hiding this comment.
I'm surprised CodexTitleProxy.address works without first waiting for CodexTitleProxy.deployed()... how does it know the deployed address? From the previous migration steps I guess?
There was a problem hiding this comment.
Yeah, from the previous migration step. We were already doing this in one other location, just decided to do it here because it's a bit cleaner
test/helpers/modifyMetadataHashes.js
Outdated
|
|
||
| // If fees are enabled, a Transfer event is fired in addition to the Modified event | ||
| const expectedLogsLength = feesEnabled ? 2 : 1 | ||
| const logIndex = feesEnabled ? 1 : 0 |
There was a problem hiding this comment.
This should maybe be something like modifiedLogIndex instead of just logIndex.
test/token/CodexTitle.test.js
Outdated
| hashedMetadata: { | ||
| name: web3.sha3('First token'), | ||
| description: web3.sha3('This is the first token'), | ||
| images: ['asdf'].map((image) => { |
There was a problem hiding this comment.
Maybe just make this:
images: [web3.sha3('image data')],Since you're not using the un-hashed versions anywhere.
test/token/CodexTitleProxy.test.js
Outdated
| hashedMetadata: { | ||
| name: web3.sha3('First token'), | ||
| description: web3.sha3('This is the first token'), | ||
| images: ['asdf'].map((image) => { |
|
Will merge & rebase when addressing comments |
| * @title ERC900BasicStakeContainer | ||
| */ | ||
| contract ERC900BasicStakeContainer is ERC900 { | ||
| using SafeMath for uint256; |
There was a problem hiding this comment.
Some gas optimization is possible here if we deploy the library separately and then point to it (as opposed to bundling it with both contracts)
| } | ||
|
|
||
| require( | ||
| codexToken.transferFrom(msg.sender, feeRecipient, calculatedFee), |
There was a problem hiding this comment.
The docs for transferFrom say Usage of this method is discouraged, use safeTransferFrom whenever possible which seems to safely handle transfers to contracts. Should we use that instead?
There was a problem hiding this comment.
We already know at this point the user who sent the tokens can hold tokens, so no harm in using transfer directly here and saving a bit of gas.
Background
In the white paper we talk at length about the ability for users to stake tokens to receive reduced discounts on the protocol fees. This is a proof of concept implementation of how this will work.
In the current implementation, rather than receive a discount for the weight of a stake for a given address (this will be calculated by the time + size of stakes), it's a binary operation that disables fees.
Updates
Future work