Skip to content
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

Fix transferData not being passed correctly #59

Merged
merged 1 commit into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
24 changes: 17 additions & 7 deletions contracts/mocks/ERC1155ReceiverMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,29 @@ contract ERC1155ReceiverMock {
* This function MAY throw to revert and reject the transfer.
* Return of other than the magic value MUST result in the transaction being reverted.
* Note: The contract address is always the message sender.
* @param _operator The address which called the `safeTransferFrom` function
* @param _from The address which previously owned the token
* @param _id The id of the token being transferred
* @param _value The amount of tokens being transferred
* @param _data Additional data with no specified format
* @return `bytes4(keccak256("onERC1155Received(address,address,uint256,uint256,bytes)"))`
*/
function onERC1155Received(address _operator, address _from, uint256 _id, uint256 _value, bytes memory _data)
function onERC1155Received(address, address _from, uint256 _id, uint256, bytes memory _data)
public returns(bytes4)
{
// To check the following conditions;
// All the balances in the transfer MUST have been updated to match the senders intent before any hook is called on a recipient.
// All the transfer events for the transfer MUST have been emitted to reflect the balance changes before any hook is called on a recipient.
// If data is passed, must be specific
uint256 fromBalance = IERC1155(msg.sender).balanceOf(_from, _id);
uint256 toBalance = IERC1155(msg.sender).balanceOf(address(this), _id);
emit TransferSingleReceiver(_from, address(this), fromBalance, toBalance);

if (_data.length != 0) {
require(
keccak256(_data) == keccak256(abi.encodePacked("Hello from the other side")),
"ERC1155ReceiverMock#onERC1155Received: UNEXPECTED_DATA"
);
}

if (shouldReject == true) {
return ERC1155_RECEIVED_INVALID; // Some random value
} else {
Expand All @@ -68,19 +74,18 @@ contract ERC1155ReceiverMock {
* This function MAY throw to revert and reject the transfer.
* Return of other than the magic value WILL result in the transaction being reverted.
* Note: The contract address is always the message sender.
* @param _operator The address which called the `safeBatchTransferFrom` function
* @param _from The address which previously owned the token
* @param _ids An array containing ids of each token being transferred
* @param _values An array containing amounts of each token being transferred
* @param _data Additional data with no specified format
* @return `bytes4(keccak256("onERC1155BatchReceived(address,address,uint256[],uint256[],bytes)"))`
*/
function onERC1155BatchReceived(address _operator, address _from, uint256[] memory _ids, uint256[] memory _values, bytes memory _data)
function onERC1155BatchReceived(address, address _from, uint256[] memory _ids, uint256[] memory, bytes memory _data)
public returns(bytes4)
{
// To check the following conditions;
// All the balances in the transfer MUST have been updated to match the senders intent before any hook is called on a recipient.
// All the transfer events for the transfer MUST have been emitted to reflect the balance changes before any hook is called on a recipient.
// If data is passed, must be specific
address[] memory fromAddressArray = new address[](_ids.length);
address[] memory toAddressArray = new address[](_ids.length);
for (uint i = 0; i < _ids.length; i++ ) {
Expand All @@ -91,14 +96,19 @@ contract ERC1155ReceiverMock {
uint256[] memory toBalances = IERC1155(msg.sender).balanceOfBatch(toAddressArray, _ids);
emit TransferBatchReceiver(_from, address(this), fromBalances, toBalances);

if (_data.length != 0) {
require(
keccak256(_data) == keccak256(abi.encodePacked("Hello from the other side")),
"ERC1155ReceiverMock#onERC1155Received: UNEXPECTED_DATA");
}

if (shouldReject == true) {
return ERC1155_RECEIVED_INVALID; // Some random value
} else {
return ERC1155_BATCH_RECEIVED_SIG;
}
}


/***********************************|
| ERC165 Functions |
|__________________________________*/
Expand Down
4 changes: 2 additions & 2 deletions contracts/tokens/ERC1155/ERC1155Meta.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ contract ERC1155Meta is ERC1155, SignatureValidator {
// Hence we only pass the gasLimit to the recipient such that the relayer knows the griefing
// limit. Nothing can prevent the receiver to revert the transaction as close to the gasLimit as
// possible, but the relayer can now only accept meta-transaction gasLimit within a certain range.
_callonERC1155Received(_from, _to, _id, _amount, gasReceipt.gasLimitCallback, signedData);
_callonERC1155Received(_from, _to, _id, _amount, gasReceipt.gasLimitCallback, transferData);

// Transfer gas cost
_transferGasFee(_from, gasReceipt);
Expand Down Expand Up @@ -173,7 +173,7 @@ contract ERC1155Meta is ERC1155, SignatureValidator {
// Hence we only pass the gasLimit to the recipient such that the relayer knows the griefing
// limit. Nothing can prevent the receiver to revert the transaction as close to the gasLimit as
// possible, but the relayer can now only accept meta-transaction gasLimit within a certain range.
_callonERC1155BatchReceived(_from, _to, _ids, _amounts, gasReceipt.gasLimitCallback, signedData);
_callonERC1155BatchReceived(_from, _to, _ids, _amounts, gasReceipt.gasLimitCallback, transferData);

// Handle gas reimbursement
_transferGasFee(_from, gasReceipt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ contract ERC1155MetaPackedBalance is ERC1155PackedBalance, SignatureValidator {
// Hence we only pass the gasLimit to the recipient such that the relayer knows the griefing
// limit. Nothing can prevent the receiver to revert the transaction as close to the gasLimit as
// possible, but the relayer can now only accept meta-transaction gasLimit within a certain range.
_callonERC1155Received(_from, _to, _id, _amount, gasReceipt.gasLimitCallback, signedData);
_callonERC1155Received(_from, _to, _id, _amount, gasReceipt.gasLimitCallback, transferData);

// Transfer gas cost
_transferGasFee(_from, gasReceipt);
Expand Down Expand Up @@ -176,7 +176,7 @@ contract ERC1155MetaPackedBalance is ERC1155PackedBalance, SignatureValidator {
// Hence we only pass the gasLimit to the recipient such that the relayer knows the griefing
// limit. Nothing can prevent the receiver to revert the transaction as close to the gasLimit as
// possible, but the relayer can now only accept meta-transaction gasLimit within a certain range.
_callonERC1155BatchReceived(_from, _to, _ids, _amounts, gasReceipt.gasLimitCallback, signedData);
_callonERC1155BatchReceived(_from, _to, _ids, _amounts, gasReceipt.gasLimitCallback, transferData);

// Handle gas reimbursement
_transferGasFee(_from, gasReceipt);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "multi-token-standard",
"version": "0.9.5",
"version": "0.9.6",
"description": "ERC1155 Multi-Token Standard implementation for Ethereum",
"repository": "https://github.com/horizon-games/multi-token-standard",
"license": "MIT",
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ERC1155.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ contract('ERC1155', (accounts: string[]) => {
})

it('should pass if data is not null to receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// NOTE: typechain generates the wrong type for `bytes` type at this time
// see https://github.com/ethereum-ts/TypeChain/issues/123
Expand Down Expand Up @@ -399,7 +399,7 @@ contract('ERC1155', (accounts: string[]) => {
})

it('should pass if data is not null from receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// TODO: remove ts-ignore when contract declaration is fixed
// @ts-ignore
Expand Down
2 changes: 0 additions & 2 deletions src/tests/ERC1155Meta.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,6 @@ contract('ERC1155Meta', (accounts: string[]) => {

const tx = operatorERC1155Contract.functions.metaSafeBatchTransferFrom(ownerAddress, receiverContract.address, ids, amounts, isGasReceipt, data)
await expect(tx).to.be.rejectedWith( RevertError("ERC1155#_callonERC1155BatchReceived: INVALID_ON_RECEIVE_MESSAGE") )

})

it('should PASS if valid response from receiver contract', async () => {
Expand All @@ -1207,7 +1206,6 @@ contract('ERC1155Meta', (accounts: string[]) => {
{gasLimit: 2000000}
)

//await expect(tx).to.be.fulfilled
await expect(tx).to.be.fulfilled
})

Expand Down
4 changes: 2 additions & 2 deletions src/tests/ERC1155MintBurn.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ contract('ERC1155MintBurn', (accounts: string[]) => {
})

it('should pass if data is not null to receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// NOTE: typechain generates the wrong type for `bytes` type at this time
// see https://github.com/ethereum-ts/TypeChain/issues/123
Expand Down Expand Up @@ -249,7 +249,7 @@ contract('ERC1155MintBurn', (accounts: string[]) => {
})

it('should pass if data is not null from receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello123')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// TODO: remove ts-ignore when contract declaration is fixed
// @ts-ignore
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ERC1155MintBurnPackedBalance.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ contract('ERC1155MintBurnPackedBalance', (accounts: string[]) => {
})

it('should pass if data is not null to receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// NOTE: typechain generates the wrong type for `bytes` type at this time
// see https://github.com/ethereum-ts/TypeChain/issues/123
Expand Down Expand Up @@ -317,7 +317,7 @@ contract('ERC1155MintBurnPackedBalance', (accounts: string[]) => {
})

it('should pass if data is not null from receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello123')
const data = ethers.utils.toUtf8Bytes('Hello from the other side')

// TODO: remove ts-ignore when contract declaration is fixed
// @ts-ignore
Expand Down
4 changes: 2 additions & 2 deletions src/tests/ERC1155PackedBalance.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ contract('ERC1155PackedBalance', (accounts: string[]) => {
})

it('should pass if data is not null from receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// NOTE: typechain generates the wrong type for `bytes` type at this time
// see https://github.com/ethereum-ts/TypeChain/issues/123
Expand Down Expand Up @@ -424,7 +424,7 @@ contract('ERC1155PackedBalance', (accounts: string[]) => {
})

it('should pass if data is not null from receiver contract', async () => {
const data = ethers.utils.toUtf8Bytes('hello')
const data = ethers.utils.toUtf8Bytes("Hello from the other side")

// TODO: remove ts-ignore when contract declaration is fixed
// @ts-ignore
Expand Down
10 changes: 5 additions & 5 deletions src/tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export async function encodeMetaTransferFromData(s: TransferSignature, domainHas

// 2.
} else {
let gasAndTransferData = defaultAbiCoder.encode([GasReceiptType, 'bytes'], [gasReceipt, toUtf8Bytes('')])
let gasAndTransferData = defaultAbiCoder.encode([GasReceiptType, 'bytes'], [gasReceipt, []])
sigData = ethers.utils.keccak256(ethers.utils.solidityPack(
['bytes', 'bytes32'],
[sigData, ethers.utils.keccak256(gasAndTransferData)] //Hash of _data
Expand All @@ -159,7 +159,7 @@ export async function encodeMetaTransferFromData(s: TransferSignature, domainHas

// 4.
} else {
let emptyTransferData = defaultAbiCoder.encode(['bytes'], [toUtf8Bytes('')])
let emptyTransferData = []
sigData = ethers.utils.keccak256(ethers.utils.solidityPack(
['bytes', 'bytes32'],
[sigData, ethers.utils.keccak256(emptyTransferData)] //Hash of _data
Expand Down Expand Up @@ -240,7 +240,7 @@ export async function encodeMetaBatchTransferFromData(s: BatchTransferSignature,

// 2.
} else {
let gasAndTransferData = defaultAbiCoder.encode([GasReceiptType, 'bytes'], [gasReceipt, toUtf8Bytes('')])
let gasAndTransferData = defaultAbiCoder.encode([GasReceiptType, 'bytes'], [gasReceipt, []])
sigData = ethers.utils.keccak256(ethers.utils.solidityPack(
['bytes', 'bytes32'],
[sigData, ethers.utils.keccak256(gasAndTransferData)] //Hash of _data
Expand All @@ -262,7 +262,7 @@ export async function encodeMetaBatchTransferFromData(s: BatchTransferSignature,

// 4.
} else {
let emptyTransferData = defaultAbiCoder.encode(['bytes'], [toUtf8Bytes('')])
let emptyTransferData = []
sigData = ethers.utils.keccak256(ethers.utils.solidityPack(
['bytes', 'bytes32'],
[sigData, ethers.utils.keccak256(emptyTransferData)] //Hash of _data
Expand Down Expand Up @@ -321,7 +321,7 @@ export async function encodeMetaApprovalData(a: ApprovalSignature, domainHash: s
return defaultAbiCoder.encode(txDataTypes, [sig, gasData])

} else {
let emptyTransferData = defaultAbiCoder.encode(['bytes'], [toUtf8Bytes('')])
let emptyTransferData = []
sigData = ethers.utils.keccak256(ethers.utils.solidityPack(
['bytes', 'bytes32'],
[sigData, ethers.utils.keccak256(emptyTransferData)] //Hash of _data
Expand Down
Loading