-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ERC827-like methods for ERC721Holdings #2
base: master
Are you sure you want to change the base?
Conversation
contracts/mocks/MessageHelper.sol
Outdated
@@ -0,0 +1,31 @@ | |||
pragma solidity ^0.4.21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's be consistent and use 'pragma solidity ^0.4.23;'
contracts/mocks/MessageHelper.sol
Outdated
return false; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline to EOF
|
||
describe('Test Execute Calls methods', function () { | ||
it('should allow payment through approve', async function () { | ||
const extraData = this.message.contract.buyMessage.getData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used encodeCall
helper in r8-app tests.
It would look something like this:
const data = encodeCall('buyMessage', ['bytes32', 'uint256', 'string'], [web3.toHex(123456), 666, 'Transfer Done']);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are using the web3 v0.20 API and this will change with web3 v1.0 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a FYI comment, no need to change anything for now.
}); | ||
}); | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline to EOF
|
||
function transferFromAndCall(address _from, uint256 _to, address _toOrigin, uint256 _tokenId, bytes _data) | ||
public payable returns (bool); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline at EOF
This pull request adds ERC827-like methods to ERC721Holdings to allow for value transfer with data transfer.