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

fix: add minor updates and improvements #22

Merged
merged 3 commits into from
Apr 15, 2024
Merged

fix: add minor updates and improvements #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39ece70
fix: use dynamic sbt name, symbol in initialize
yashdesu Apr 9, 2024
e93e5ba
fix: remove onlyOwner from makeNewSBT
yashdesu Apr 9, 2024
2ed65ba
fix: update test cases as per smart-contract upgrades
yashdesu Apr 9, 2024
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
4 changes: 3 additions & 1 deletion contracts/SBT.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ contract SBT is ISBT, ERC721EnumerableUpgradeable {
}

function initialize(
string memory sbtName,
string memory sbtSymbol,
address minterUpdater,
address[] memory minters
) external initializer {
__ERC721_init("Dev Protocol SBT V1", "DEV-SBT-V1");
__ERC721_init(sbtName, sbtSymbol);

_minterUpdater = minterUpdater;
for (uint256 i = 0; i < minters.length; i++) {
Expand Down
6 changes: 4 additions & 2 deletions contracts/SBTFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ contract SBTFactory is ISBTFactory, OwnableUpgradeable {
}

function makeNewSBT(
string memory sbtName,
string memory sbtSymbol,
address proxyAdmin,
bytes memory proxyCallData,
address minterUpdater,
address[] calldata minters,
bytes calldata identifier
) external override onlyOwner returns (address) {
) external override returns (address) {
require(
sbtProxyMapping[identifier] == address(0),
"Identifier already used"
Expand All @@ -52,7 +54,7 @@ contract SBTFactory is ISBTFactory, OwnableUpgradeable {
sbtProxyMapping[identifier] = address(proxy);

// Initialize the proxy.
SBT(proxy).initialize(minterUpdater, minters);
SBT(proxy).initialize(sbtName, sbtSymbol, minterUpdater, minters);

return address(proxy);
}
Expand Down
4 changes: 4 additions & 0 deletions contracts/interfaces/ISBTFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pragma solidity =0.8.9;
interface ISBTFactory {
/*
* @dev makes new SBT contract which is upgradeable.
* @param sbtName The name of the SBT token to be created.
* @param sbtSymbol The symbol of the SBT token to be created.
* @param proxyAdmin owner of the proxy contract which will be deployed.
* @param proxyCallData the data which denotes function calls, etc when deploying new proxy contract.
* @param minterUpdater the address which can add/remove minter access eoa.
Expand All @@ -12,6 +14,8 @@ interface ISBTFactory {
* @return uint256[] token id list
*/
function makeNewSBT(
string memory sbtName,
string memory sbtSymbol,
address proxyAdmin,
bytes memory proxyCallData,
address minterUpdater,
Expand Down
4 changes: 2 additions & 2 deletions test/SBT.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('SBT', () => {
const init = async (): Promise<Contract> => {
const signers = await getSigners()
const sbt = await deploy('SBT')
await sbt.initialize(signers.minterUpdater.address, [
await sbt.initialize('Test SBT', 'TESTSBT', signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
Expand All @@ -22,7 +22,7 @@ describe('SBT', () => {
it('The initialize function can only be executed once', async () => {
const sbt = await init()
await expect(
sbt.initialize(constants.AddressZero, [
sbt.initialize('Test SBT', 'TESTSBT', constants.AddressZero, [
constants.AddressZero,
constants.AddressZero,
])
Expand Down
16 changes: 13 additions & 3 deletions test/SBTFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ describe('SBTFactory', () => {
expect(await sbtFactory.sbtProxyMapping(identifier)).to.eq(ZERO_ADDRESS)
await expect(
sbtFactory.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
Expand All @@ -72,6 +74,8 @@ describe('SBTFactory', () => {
sbtFactory
.connect(signers.deployer)
.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
Expand All @@ -87,7 +91,7 @@ describe('SBTFactory', () => {
)
})

it('The makeNewSBT function should not execute if called by non-owner', async () => {
it('The makeNewSBT function should execute if called by non-owner', async () => {
const signers = await getSigners()
const sbtFactory = await init()

Expand All @@ -97,15 +101,21 @@ describe('SBTFactory', () => {
sbtFactory
.connect(signers.minterA)
.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
[signers.minterA.address, signers.minterB.address],
identifier
)
).to.revertedWith('Ownable: caller is not the owner')
)
.to.emit(sbtFactory, 'SBTImplementationCreated')
.emit(sbtFactory, 'SBTProxyCreated')

expect(await sbtFactory.sbtProxyMapping(identifier)).to.eq(ZERO_ADDRESS)
expect(await sbtFactory.sbtProxyMapping(identifier)).to.not.eq(
ZERO_ADDRESS
)
})
})

Expand Down
16 changes: 13 additions & 3 deletions test/SBTFactoryProxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ describe('SBTFactoryProxy', () => {
expect(await sbtFactory.sbtProxyMapping(identifier)).to.eq(ZERO_ADDRESS)
await expect(
sbtFactory.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
Expand All @@ -514,6 +516,8 @@ describe('SBTFactoryProxy', () => {
sbtFactory
.connect(signers.deployer)
.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
Expand All @@ -529,7 +533,7 @@ describe('SBTFactoryProxy', () => {
)
})

it('The makeNewSBT function should not execute if called by non-owner', async () => {
it('The makeNewSBT function should execute if called by non-owner', async () => {
const signers = await getSigners()
const { sbtFactory } = await init()

Expand All @@ -539,15 +543,21 @@ describe('SBTFactoryProxy', () => {
sbtFactory
.connect(signers.minterA)
.makeNewSBT(
'Test SBT',
'TESTSBT',
signers.proxyAdmin.address,
ethers.utils.arrayify('0x'),
signers.minterUpdater.address,
[signers.minterA.address, signers.minterB.address],
identifier
)
).to.revertedWith('Ownable: caller is not the owner')
)
.to.emit(sbtFactory, 'SBTImplementationCreated')
.emit(sbtFactory, 'SBTProxyCreated')

expect(await sbtFactory.sbtProxyMapping(identifier)).to.eq(ZERO_ADDRESS)
expect(await sbtFactory.sbtProxyMapping(identifier)).to.not.eq(
ZERO_ADDRESS
)
})
})

Expand Down
24 changes: 15 additions & 9 deletions test/SBTProxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ describe('SBTProxy', () => {
])
const sbt = sbtImplementation.attach(sbtProxy.address)
if (shouldAlsoInitializeProxy) {
await sbt.initialize(signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
await sbt.initialize(
'Test SBT',
'TESTSBT',
signers.minterUpdater.address,
[signers.minterA.address, signers.minterB.address]
)
}

return { sbt, sbtImplementation, sbtProxy, sbtImplementationB }
Expand Down Expand Up @@ -316,6 +318,8 @@ describe('SBTProxy', () => {
const encodedData = sbtImplementationB
.connect(signers.minterUpdater)
.interface.encodeFunctionData('initialize', [
'Test SBT',
'TESTSBT',
signers.minterUpdater.address,
[signers.minterA.address, signers.minterB.address],
])
Expand All @@ -330,7 +334,7 @@ describe('SBTProxy', () => {
await expect(
sbt
.connect(signers.userA)
.initialize(signers.minterUpdater.address, [
.initialize('Test SBT', 'TESTSBT', signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
Expand All @@ -353,6 +357,8 @@ describe('SBTProxy', () => {
const encodedData = sbtImplementationB
.connect(signers.minterUpdater)
.interface.encodeFunctionData('initialize', [
'Test SBT',
'TESTSBT',
signers.minterUpdater.address,
[signers.minterA.address, signers.minterB.address],
])
Expand All @@ -366,15 +372,15 @@ describe('SBTProxy', () => {
await expect(
sbt
.connect(signers.userA)
.initialize(signers.minterUpdater.address, [
.initialize('Test SBT', 'TESTSBT', signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
).to.not.reverted
await expect(
sbt
.connect(signers.userA)
.initialize(signers.minterUpdater.address, [
.initialize('Test SBT', 'TESTSBT', signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
Expand Down Expand Up @@ -442,7 +448,7 @@ describe('SBTProxy', () => {
await expect(
sbt
.connect(proxyAdmin)
.initialize(constants.AddressZero, [
.initialize('Test SBT', 'TESTSBT', constants.AddressZero, [
constants.AddressZero,
constants.AddressZero,
])
Expand All @@ -454,7 +460,7 @@ describe('SBTProxy', () => {
it('The initialize function can only be executed once', async () => {
const { sbt } = await init()
await expect(
sbt.initialize(constants.AddressZero, [
sbt.initialize('Test SBT', 'TESTSBT', constants.AddressZero, [
constants.AddressZero,
constants.AddressZero,
])
Expand Down
2 changes: 1 addition & 1 deletion test/SBTTokenURI.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('SBT', () => {
const init = async (): Promise<Contract> => {
const signers = await getSigners()
const sbt = await deploy('SBT')
await sbt.initialize(signers.minterUpdater.address, [
await sbt.initialize('Test SBT', 'TESTSBT', signers.minterUpdater.address, [
signers.minterA.address,
signers.minterB.address,
])
Expand Down