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.

Sign up for GitHub

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

Jump to bottom

Update to latest OpenZeppelin base contracts #147

Closed
wants to merge 1 commit into from
Closed

Update to latest OpenZeppelin base contracts #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 samples/solidity/contracts/ERC20NoData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import '@openzeppelin/contracts/access/Ownable.sol';
* This is a sample only and NOT a reference implementation.
*/
contract ERC20NoData is Context, Ownable, ERC20, ERC20Burnable {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
constructor(string memory name, string memory symbol) ERC20(name, symbol) Ownable(msg.sender) {}

function mint(address to, uint256 amount) public onlyOwner {
_mint(to, amount);
Expand Down
2 changes: 1 addition & 1 deletion samples/solidity/contracts/ERC20WithData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import './IERC20WithData.sol';
* This is a sample only and NOT a reference implementation.
*/
contract ERC20WithData is Context, Ownable, ERC165, ERC20, IERC20WithData {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
constructor(string memory name, string memory symbol) ERC20(name, symbol) Ownable(msg.sender) {}

function supportsInterface(
bytes4 interfaceId
Expand Down
16 changes: 6 additions & 10 deletions samples/solidity/contracts/ERC721NoData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import '@openzeppelin/contracts/token/ERC721/ERC721.sol';
import '@openzeppelin/contracts/token/ERC721/extensions/ERC721Burnable.sol';
import '@openzeppelin/contracts/utils/Context.sol';
import '@openzeppelin/contracts/access/Ownable.sol';
import '@openzeppelin/contracts/utils/Counters.sol';

/**
* Example ERC721 token with mint and burn.
Expand All @@ -19,18 +18,15 @@ import '@openzeppelin/contracts/utils/Counters.sol';
* This is a sample only and NOT a reference implementation.
*/
contract ERC721NoData is Context, Ownable, ERC721, ERC721Burnable {
using Counters for Counters.Counter;
uint256 private _nextTokenId = 1;

Counters.Counter private _tokenIdCounter;

constructor(string memory name, string memory symbol) ERC721(name, symbol) {
// Start counting at 1
_tokenIdCounter.increment();
}
constructor(
string memory name,
string memory symbol
) ERC721(name, symbol) Ownable(msg.sender) {}

function safeMint(address to) public onlyOwner {
uint256 tokenId = _tokenIdCounter.current();
_tokenIdCounter.increment();
uint256 tokenId = _nextTokenId++;
_safeMint(to, tokenId);
}

Expand Down
20 changes: 6 additions & 14 deletions samples/solidity/contracts/ERC721WithData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.0;
import '@openzeppelin/contracts/access/Ownable.sol';
import '@openzeppelin/contracts/utils/Context.sol';
import '@openzeppelin/contracts/utils/Strings.sol';
import '@openzeppelin/contracts/utils/Counters.sol';
import '@openzeppelin/contracts/token/ERC721/ERC721.sol';
import './IERC721WithData.sol';

Expand All @@ -25,9 +24,7 @@ import './IERC721WithData.sol';
* This is a sample only and NOT a reference implementation.
*/
contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
using Counters for Counters.Counter;

Counters.Counter private _tokenIdCounter;
uint256 private _nextTokenId = 1;
string private _baseTokenURI;

// Optional mapping for token URIs
Expand All @@ -37,10 +34,8 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
string memory name,
string memory symbol,
string memory baseTokenURI
) ERC721(name, symbol) {
) ERC721(name, symbol) Ownable(msg.sender) {
_baseTokenURI = baseTokenURI;
// Start counting at 1
_tokenIdCounter.increment();
}

function supportsInterface(
Expand All @@ -52,8 +47,7 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
}

function mintWithData(address to, bytes calldata data) public virtual onlyOwner {
uint256 tokenId = _tokenIdCounter.current();
_tokenIdCounter.increment();
uint256 tokenId = _nextTokenId++;
_safeMint(to, tokenId, data);
_setTokenURI(tokenId, string(abi.encodePacked(_baseURI(), Strings.toString(tokenId))));
}
Expand All @@ -63,8 +57,7 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
bytes calldata data,
string memory tokenURI_
) public virtual onlyOwner {
uint256 tokenId = _tokenIdCounter.current();
_tokenIdCounter.increment();
uint256 tokenId = _nextTokenId++;
_safeMint(to, tokenId, data);

// If there is no tokenURI passed, concatenate the tokenID to the base URI
Expand Down Expand Up @@ -112,14 +105,13 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
}

function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
require(_exists(tokenId), 'ERC721WithData: Token does not exist');

_requireOwned(tokenId);
string memory uri = _tokenURIs[tokenId];
return uri;
}

function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal {
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 didn't do it here, but I wonder if we should be inheriting https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/extensions/ERC721URIStorage.sol

require(_exists(tokenId), 'ERC721WithData: Token does not exist');
_requireOwned(tokenId);
_tokenURIs[tokenId] = _tokenURI;
}

Expand Down
8 changes: 4 additions & 4 deletions samples/solidity/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion samples/solidity/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"typescript": "^5.3.3"
},
"dependencies": {
"@openzeppelin/contracts": "^4.7.3"
"@openzeppelin/contracts": "^5.0.2"
}
}
14 changes: 7 additions & 7 deletions samples/solidity/test/ERC20NoData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers';
import { expect } from "chai";
import { ethers } from "hardhat";
import { expect } from 'chai';
import { ethers } from 'hardhat';
import { ERC20NoData } from '../typechain-types';

describe('ERC20NoData - Unit Tests', async function () {
Expand Down Expand Up @@ -43,9 +43,9 @@ describe('ERC20NoData - Unit Tests', async function () {
it('Mint - Non-deployer of contract should not be able to mint tokens', async function () {
expect(await deployedERC20NoData.balanceOf(signerB.address)).to.equal(0);
// Signer B mint to Signer B (Not allowed)
await expect(deployedERC20NoData.connect(signerB).mint(signerB.address, 20)).to.be.revertedWith(
'Ownable: caller is not the owner',
);
await expect(
deployedERC20NoData.connect(signerB).mint(signerB.address, 20),
).to.be.revertedWithCustomError(deployedERC20NoData, 'OwnableUnauthorizedAccount');

expect(await deployedERC20NoData.balanceOf(signerB.address)).to.equal(0);
});
Expand Down Expand Up @@ -162,11 +162,11 @@ describe('ERC20NoData - Unit Tests', async function () {
// Signer B attempts to burn tokens from Signer A wallet (not allowed)
await expect(
deployedERC20NoData.connect(signerB).burnFrom(deployerSignerA.address, 10),
).to.be.revertedWith('ERC20: insufficient allowance');
).to.be.revertedWithCustomError(deployedERC20NoData, 'ERC20InsufficientAllowance');
// Signer A attempts to burn tokens from Signer B wallet (not allowed)
await expect(
deployedERC20NoData.connect(deployerSignerA).burnFrom(signerB.address, 10),
).to.be.revertedWith('ERC20: insufficient allowance');
).to.be.revertedWithCustomError(deployedERC20NoData, 'ERC20InsufficientAllowance');

expect(await deployedERC20NoData.balanceOf(deployerSignerA.address)).to.equal(20);
expect(await deployedERC20NoData.balanceOf(signerB.address)).to.equal(20);
Expand Down
6 changes: 3 additions & 3 deletions samples/solidity/test/ERC20WithData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('ERC20WithData - Unit Tests', async function () {
// Signer B mint to Signer B (Not allowed)
await expect(
deployedERC20WithData.connect(signerB).mintWithData(signerB.address, 20, '0x00'),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(deployedERC20WithData, 'OwnableUnauthorizedAccount');

expect(await deployedERC20WithData.balanceOf(signerB.address)).to.equal(0);
});
Expand All @@ -57,7 +57,7 @@ describe('ERC20WithData - Unit Tests', async function () {
// Signer B mint to Signer B (Not allowed)
await expect(
deployedERC20WithData.connect(signerB).mintWithData(signerB.address, 20, '0x00'),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(deployedERC20WithData, 'OwnableUnauthorizedAccount');

expect(await deployedERC20WithData.balanceOf(signerB.address)).to.equal(0);
});
Expand Down Expand Up @@ -142,7 +142,7 @@ describe('ERC20WithData - Unit Tests', async function () {
deployedERC20WithData
.connect(deployerSignerA)
.transferWithData(signerB.address, signerC.address, 11, '0x00'),
).to.be.revertedWith('ERC20: insufficient allowance');
).to.be.revertedWithCustomError(deployedERC20WithData, 'ERC20InsufficientAllowance');

expect(await deployedERC20WithData.balanceOf(deployerSignerA.address)).to.equal(0);
expect(await deployedERC20WithData.balanceOf(signerB.address)).to.equal(20);
Expand Down
16 changes: 9 additions & 7 deletions samples/solidity/test/ERC721NoData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('ERC721NoData - Unit Tests', async function () {
// Signer B mint to Signer B (Not allowed)
await expect(
deployedERC721NoData.connect(signerB).safeMint(signerB.address),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(deployedERC721NoData, 'OwnableUnauthorizedAccount');

expect(await deployedERC721NoData.balanceOf(signerB.address)).to.equal(0);
});
Expand All @@ -56,7 +56,7 @@ describe('ERC721NoData - Unit Tests', async function () {
// Signer B mint token to Signer B (Not allowed)
await expect(
deployedERC721NoData.connect(signerB).safeMint(signerB.address),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(deployedERC721NoData, 'OwnableUnauthorizedAccount');

expect(await deployedERC721NoData.balanceOf(signerB.address)).to.equal(0);
});
Expand Down Expand Up @@ -136,7 +136,7 @@ describe('ERC721NoData - Unit Tests', async function () {
deployedERC721NoData
.connect(deployerSignerA)
['safeTransferFrom(address,address,uint256)'](signerB.address, signerC.address, 1),
).to.be.revertedWith('ERC721: caller is not token owner or approved');
).to.be.revertedWithCustomError(deployedERC721NoData, 'ERC721InsufficientApproval');

expect(await deployedERC721NoData.balanceOf(deployerSignerA.address)).to.equal(0);
expect(await deployedERC721NoData.balanceOf(signerB.address)).to.equal(2);
Expand Down Expand Up @@ -213,12 +213,14 @@ describe('ERC721NoData - Unit Tests', async function () {
.to.emit(deployedERC721NoData, 'Transfer')
.withArgs(ZERO_ADDRESS, signerC.address, 3);
// Signer B attempts to burn token from Signer A wallet (not allowed)
await expect(deployedERC721NoData.connect(signerB).burn(1)).to.be.revertedWith(
'ERC721: caller is not token owner or approved',
await expect(deployedERC721NoData.connect(signerB).burn(1)).to.be.revertedWithCustomError(
deployedERC721NoData,
'ERC721InsufficientApproval',
);
// Signer C attempts to burn token from Signer B wallet (not allowed)
await expect(deployedERC721NoData.connect(signerC).burn(2)).to.be.revertedWith(
'ERC721: caller is not token owner or approved',
await expect(deployedERC721NoData.connect(signerC).burn(2)).to.be.revertedWithCustomError(
deployedERC721NoData,
'ERC721InsufficientApproval',
);

expect(await deployedERC721NoData.balanceOf(deployerSignerA.address)).to.equal(1);
Expand Down
4 changes: 2 additions & 2 deletions samples/solidity/test/ERC721WithData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('ERC721WithData - Unit Tests', async function () {
// Signer B mint token to Signer B (Not allowed)
await expect(
deployedERC721WithData.connect(signerB).mintWithData(signerB.address, '0x00'),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(deployedERC721WithData, 'OwnableUnauthorizedAccount');

expect(await deployedERC721WithData.balanceOf(signerB.address)).to.equal(0);
});
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('ERC721WithData - Unit Tests', async function () {
deployedERC721WithData
.connect(deployerSignerA)
.transferWithData(signerB.address, signerC.address, 1, '0x00'),
).to.be.revertedWith('ERC721: caller is not token owner or approved');
).to.be.revertedWithCustomError(deployedERC721WithData, 'ERC721InsufficientApproval');

expect(await deployedERC721WithData.balanceOf(deployerSignerA.address)).to.equal(0);
expect(await deployedERC721WithData.balanceOf(signerB.address)).to.equal(2);
Expand Down
Loading