From a821fe4b9c710123df94e7dd3698284cbf0b2f3c Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 23 Oct 2024 14:42:08 +1100 Subject: [PATCH 1/6] balanceOf reads credits into memory --- contracts/contracts/token/OUSD.sol | 7 ++++--- contracts/test/token/ousd.js | 32 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol index 5f8dba4c46..19cdcf5a5e 100644 --- a/contracts/contracts/token/OUSD.sol +++ b/contracts/contracts/token/OUSD.sol @@ -121,9 +121,10 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { override returns (uint256) { - if (_creditBalances[_account] == 0) return 0; - return - _creditBalances[_account].divPrecisely(_creditsPerToken(_account)); + // Read credits from storage + uint256 credits = _creditBalances[_account]; + if (credits == 0) return 0; + return credits.divPrecisely(_creditsPerToken(_account)); } /** diff --git a/contracts/test/token/ousd.js b/contracts/test/token/ousd.js index caea0ad059..1d5c64d0d9 100644 --- a/contracts/test/token/ousd.js +++ b/contracts/test/token/ousd.js @@ -24,6 +24,38 @@ describe("Token", function () { expect(await ousd.decimals()).to.equal(18); }); + describe("Should measure gas of balanceOf from", async () => { + it("rebasing account", async () => { + const { matt, ousd } = fixture; + + const tx = await ousd + .connect(matt) + .populateTransaction.balanceOf(matt.getAddress()); + await matt.sendTransaction(tx); + }); + + it("non rebasing account", async () => { + const { matt, ousd } = fixture; + + await ousd.connect(matt).rebaseOptOut(); + const tx = await ousd + .connect(matt) + .populateTransaction.balanceOf(matt.getAddress()); + await matt.sendTransaction(tx); + }); + + it("zero balance account", async () => { + const { matt, ousd } = fixture; + + const tx = await ousd + .connect(matt) + .populateTransaction.balanceOf( + "0x0000000000000000000000000000000000000001" + ); + await matt.sendTransaction(tx); + }); + }); + it("Should return 0 balance for the zero address", async () => { const { ousd } = fixture; expect( From 87c15814d208b40676e2295d363e90aee73493da Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 23 Oct 2024 14:52:04 +1100 Subject: [PATCH 2/6] _creditsPerToken read nonRebasingCreditsPerToken into memory --- contracts/contracts/token/OUSD.sol | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol index 19cdcf5a5e..556e75e907 100644 --- a/contracts/contracts/token/OUSD.sol +++ b/contracts/contracts/token/OUSD.sol @@ -446,11 +446,13 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { view returns (uint256) { - if (nonRebasingCreditsPerToken[_account] != 0) { - return nonRebasingCreditsPerToken[_account]; - } else { - return _rebasingCreditsPerToken; + // Read the account's non-rebasing credits per token from storage + uint256 creditsPerTokenMem = nonRebasingCreditsPerToken[_account]; + if (creditsPerTokenMem != 0) { + return creditsPerTokenMem; } + + return _rebasingCreditsPerToken; } /** From 62b14b43a422e68fdf239025e0d552770e33e889 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 23 Oct 2024 16:53:09 +1100 Subject: [PATCH 3/6] Exit early in _isNonRebasingAccount if the account is non rebasing --- contracts/contracts/token/OUSD.sol | 52 ++++++++++++++++++------------ 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol index 556e75e907..6019a82cd7 100644 --- a/contracts/contracts/token/OUSD.sol +++ b/contracts/contracts/token/OUSD.sol @@ -459,13 +459,27 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { * @dev Is an account using rebasing accounting or non-rebasing accounting? * Also, ensure contracts are non-rebasing if they have not opted in. * @param _account Address of the account. + * @return bool True if the account is using non-rebasing accounting. + * False if the account is using rebasing accounting. */ function _isNonRebasingAccount(address _account) internal returns (bool) { - bool isContract = Address.isContract(_account); - if (isContract && rebaseState[_account] == RebaseOptions.NotSet) { + // Exit early if the account has already set its non-rebasing credits per token + if (nonRebasingCreditsPerToken[_account] > 0) { + return true; + } + + // If rebase option is not set and the account is a contract + // Do the contract check first as that is cheaper than reading a storage slot + if ( + Address.isContract(_account) && + rebaseState[_account] == RebaseOptions.NotSet + ) { _ensureRebasingMigration(_account); + return true; } - return nonRebasingCreditsPerToken[_account] > 0; + + // Must be using rebasing accounting + return false; } /** @@ -473,24 +487,20 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { * supply is updated following deployment of frozen yield change. */ function _ensureRebasingMigration(address _account) internal { - if (nonRebasingCreditsPerToken[_account] == 0) { - emit AccountRebasingDisabled(_account); - if (_creditBalances[_account] == 0) { - // Since there is no existing balance, we can directly set to - // high resolution, and do not have to do any other bookkeeping - nonRebasingCreditsPerToken[_account] = 1e27; - } else { - // Migrate an existing account: - - // Set fixed credits per token for this account - nonRebasingCreditsPerToken[_account] = _rebasingCreditsPerToken; - // Update non rebasing supply - nonRebasingSupply = nonRebasingSupply.add(balanceOf(_account)); - // Update credit tallies - _rebasingCredits = _rebasingCredits.sub( - _creditBalances[_account] - ); - } + emit AccountRebasingDisabled(_account); + if (_creditBalances[_account] == 0) { + // Since there is no existing balance, we can directly set to + // high resolution, and do not have to do any other bookkeeping + nonRebasingCreditsPerToken[_account] = 1e27; + } else { + // Migrate an existing account: + + // Set fixed credits per token for this account + nonRebasingCreditsPerToken[_account] = _rebasingCreditsPerToken; + // Update non rebasing supply + nonRebasingSupply = nonRebasingSupply.add(balanceOf(_account)); + // Update credit tallies + _rebasingCredits = _rebasingCredits.sub(_creditBalances[_account]); } } From a1a3d3a9919aabe65292d04201d2e3cd839bde10 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 23 Oct 2024 16:53:41 +1100 Subject: [PATCH 4/6] Grouped the OUSD transfer tests --- contracts/test/token/ousd.js | 630 ++++++++++++++++++----------------- 1 file changed, 325 insertions(+), 305 deletions(-) diff --git a/contracts/test/token/ousd.js b/contracts/test/token/ousd.js index 1d5c64d0d9..707faf7383 100644 --- a/contracts/test/token/ousd.js +++ b/contracts/test/token/ousd.js @@ -106,338 +106,358 @@ describe("Token", function () { ).to.equal(ousdUnits("999")); }); - it("Should transfer the correct amount from a rebasing account to a non-rebasing account and set creditsPerToken", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - - // Give contract 100 OUSD from Josh - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - - const contractCreditsPerToken = await ousd.creditsBalanceOf( - mockNonRebasing.address - ); - - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - - // Credits per token should be the same for the contract - contractCreditsPerToken === - (await ousd.creditsBalanceOf(mockNonRebasing.address)); - - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); - - it("Should transfer the correct amount from a rebasing account to a non-rebasing account with previously set creditsPerToken", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - // Matt received all the yield - await expect(matt).has.an.approxBalanceOf("300.00", ousd); - // Give contract 100 OUSD from Matt - await ousd.connect(matt).transfer(mockNonRebasing.address, ousdUnits("50")); - await expect(matt).has.an.approxBalanceOf("250", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); - - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); - - it("Should transfer the correct amount from a non-rebasing account without previously set creditssPerToken to a rebasing account", async () => { - let { ousd, matt, josh, mockNonRebasing } = fixture; - - // Give contract 100 OUSD from Josh - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - await mockNonRebasing.transfer(await matt.getAddress(), ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("200.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); - - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); - - it("Should transfer the correct amount from a non-rebasing account with previously set creditsPerToken to a rebasing account", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + describe("Should transfer the correct amount from a", () => { + it("rebasing account to a non-rebasing account and set creditsPerToken", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - // Give contract 100 OUSD from Josh - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - // Matt received all the yield - await expect(matt).has.an.approxBalanceOf("300.00", ousd); - // Give contract 100 OUSD from Matt - await ousd.connect(matt).transfer(mockNonRebasing.address, ousdUnits("50")); - await expect(matt).has.an.approxBalanceOf("250", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); - // Transfer contract balance to Josh - await mockNonRebasing.transfer(await josh.getAddress(), ousdUnits("150")); - await expect(matt).has.an.approxBalanceOf("250", ousd); - await expect(josh).has.an.approxBalanceOf("150", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - it("Should transfer the correct amount from a non-rebasing account to a non-rebasing account with different previously set creditsPerToken", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing, mockNonRebasingTwo } = - fixture; - // Give contract 100 OUSD from Josh - await ousd.connect(josh).transfer(mockNonRebasing.address, ousdUnits("50")); - await expect(mockNonRebasing).has.an.approxBalanceOf("50.00", ousd); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - await ousd - .connect(josh) - .transfer(mockNonRebasingTwo.address, ousdUnits("50")); - await usdc.connect(matt).transfer(vault.address, usdcUnits("100")); - await vault.rebase(); - await mockNonRebasing.transfer(mockNonRebasingTwo.address, ousdUnits("10")); - await expect(mockNonRebasing).has.an.approxBalanceOf("40", ousd); - await expect(mockNonRebasingTwo).has.an.approxBalanceOf("60", ousd); + const contractCreditsPerToken = await ousd.creditsBalanceOf( + mockNonRebasing.address + ); - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const creditBalanceMockNonRebasing = await ousd.creditsBalanceOf( - mockNonRebasing.address - ); - const balanceMockNonRebasing = creditBalanceMockNonRebasing[0] - .mul(utils.parseUnits("1", 18)) - .div(creditBalanceMockNonRebasing[1]); - const creditBalanceMockNonRebasingTwo = await ousd.creditsBalanceOf( - mockNonRebasingTwo.address - ); - const balanceMockNonRebasingTwo = creditBalanceMockNonRebasingTwo[0] - .mul(utils.parseUnits("1", 18)) - .div(creditBalanceMockNonRebasingTwo[1]); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + + // Credits per token should be the same for the contract + contractCreditsPerToken === + (await ousd.creditsBalanceOf(mockNonRebasing.address)); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(balanceMockNonRebasing) - .add(balanceMockNonRebasingTwo); + it("rebasing account to a non-rebasing account with previously set creditsPerToken", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + // Matt received all the yield + await expect(matt).has.an.approxBalanceOf("300.00", ousd); + // Give contract 100 OUSD from Matt + await ousd + .connect(matt) + .transfer(mockNonRebasing.address, ousdUnits("50")); + await expect(matt).has.an.approxBalanceOf("250", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); + it("non-rebasing account without previously set creditsPerToken to a rebasing account", async () => { + let { ousd, matt, josh, mockNonRebasing } = fixture; + + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + await mockNonRebasing.transfer(await matt.getAddress(), ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("200.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - it("Should transferFrom the correct amount from a rebasing account to a non-rebasing account and set creditsPerToken", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + it("non-rebasing account with previously set creditsPerToken to a rebasing account", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + // Matt received all the yield + await expect(matt).has.an.approxBalanceOf("300.00", ousd); + // Give contract 100 OUSD from Matt + await ousd + .connect(matt) + .transfer(mockNonRebasing.address, ousdUnits("50")); + await expect(matt).has.an.approxBalanceOf("250", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); + // Transfer contract balance to Josh + await mockNonRebasing.transfer(await josh.getAddress(), ousdUnits("150")); + await expect(matt).has.an.approxBalanceOf("250", ousd); + await expect(josh).has.an.approxBalanceOf("150", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - // Give Josh an allowance to move Matt's OUSD - await ousd - .connect(matt) - .increaseAllowance(await josh.getAddress(), ousdUnits("100")); - // Give contract 100 OUSD from Matt via Josh - await ousd - .connect(josh) - .transferFrom( - await matt.getAddress(), - mockNonRebasing.address, - ousdUnits("100") + it("non-rebasing account to a non-rebasing account with different previously set creditsPerToken", async () => { + let { + ousd, + vault, + matt, + usdc, + josh, + mockNonRebasing, + mockNonRebasingTwo, + } = fixture; + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("50")); + await expect(mockNonRebasing).has.an.approxBalanceOf("50.00", ousd); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + await ousd + .connect(josh) + .transfer(mockNonRebasingTwo.address, ousdUnits("50")); + await usdc.connect(matt).transfer(vault.address, usdcUnits("100")); + await vault.rebase(); + await mockNonRebasing.transfer( + mockNonRebasingTwo.address, + ousdUnits("10") ); - await expect(matt).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - const contractCreditsPerToken = await ousd.creditsBalanceOf( - mockNonRebasing.address - ); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - // Credits per token should be the same for the contract - contractCreditsPerToken === - (await ousd.creditsBalanceOf(mockNonRebasing.address)); + await expect(mockNonRebasing).has.an.approxBalanceOf("40", ousd); + await expect(mockNonRebasingTwo).has.an.approxBalanceOf("60", ousd); - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const creditBalanceMockNonRebasing = await ousd.creditsBalanceOf( + mockNonRebasing.address + ); + const balanceMockNonRebasing = creditBalanceMockNonRebasing[0] + .mul(utils.parseUnits("1", 18)) + .div(creditBalanceMockNonRebasing[1]); + const creditBalanceMockNonRebasingTwo = await ousd.creditsBalanceOf( + mockNonRebasingTwo.address + ); + const balanceMockNonRebasingTwo = creditBalanceMockNonRebasingTwo[0] + .mul(utils.parseUnits("1", 18)) + .div(creditBalanceMockNonRebasingTwo[1]); + + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(balanceMockNonRebasing) + .add(balanceMockNonRebasingTwo); + + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - it("Should transferFrom the correct amount from a rebasing account to a non-rebasing account with previously set creditsPerToken", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + it("rebasing account to a non-rebasing account and set creditsPerToken", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - // Give Josh an allowance to move Matt's OUSD - await ousd - .connect(matt) - .increaseAllowance(await josh.getAddress(), ousdUnits("150")); - // Give contract 100 OUSD from Matt via Josh - await ousd - .connect(josh) - .transferFrom( - await matt.getAddress(), - mockNonRebasing.address, - ousdUnits("50") + // Give Josh an allowance to move Matt's OUSD + await ousd + .connect(matt) + .increaseAllowance(await josh.getAddress(), ousdUnits("100")); + // Give contract 100 OUSD from Matt via Josh + await ousd + .connect(josh) + .transferFrom( + await matt.getAddress(), + mockNonRebasing.address, + ousdUnits("100") + ); + await expect(matt).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + const contractCreditsPerToken = await ousd.creditsBalanceOf( + mockNonRebasing.address ); - await expect(matt).has.an.approxBalanceOf("50", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("50", ousd); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - // Give contract 50 more OUSD from Matt via Josh - await ousd - .connect(josh) - .transferFrom( - await matt.getAddress(), - mockNonRebasing.address, - ousdUnits("50") + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + // Credits per token should be the same for the contract + contractCreditsPerToken === + (await ousd.creditsBalanceOf(mockNonRebasing.address)); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() ); - await expect(mockNonRebasing).has.an.approxBalanceOf("100", ousd); - - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); + }); - it("Should transferFrom the correct amount from a non-rebasing account without previously set creditsPerToken to a rebasing account", async () => { - let { ousd, matt, josh, mockNonRebasing } = fixture; + it("rebasing account to a non-rebasing account with previously set creditsPerToken", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - // Give contract 100 OUSD from Josh - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - await mockNonRebasing.increaseAllowance( - await matt.getAddress(), - ousdUnits("100") - ); + // Give Josh an allowance to move Matt's OUSD + await ousd + .connect(matt) + .increaseAllowance(await josh.getAddress(), ousdUnits("150")); + // Give contract 100 OUSD from Matt via Josh + await ousd + .connect(josh) + .transferFrom( + await matt.getAddress(), + mockNonRebasing.address, + ousdUnits("50") + ); + await expect(matt).has.an.approxBalanceOf("50", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("50", ousd); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + // Give contract 50 more OUSD from Matt via Josh + await ousd + .connect(josh) + .transferFrom( + await matt.getAddress(), + mockNonRebasing.address, + ousdUnits("50") + ); + await expect(mockNonRebasing).has.an.approxBalanceOf("100", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - await ousd - .connect(matt) - .transferFrom( - mockNonRebasing.address, + it("non-rebasing account without previously set creditsPerToken to a rebasing account", async () => { + let { ousd, matt, josh, mockNonRebasing } = fixture; + + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + await mockNonRebasing.increaseAllowance( await matt.getAddress(), ousdUnits("100") ); - await expect(matt).has.an.approxBalanceOf("200.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); - }); - - it("Should transferFrom the correct amount from a non-rebasing account with previously set creditsPerToken to a rebasing account", async () => { - let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; - - // Give contract 100 OUSD from Josh - await ousd - .connect(josh) - .transfer(mockNonRebasing.address, ousdUnits("100")); - await expect(matt).has.an.approxBalanceOf("100.00", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); - // Transfer USDC into the Vault to simulate yield - await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); - await vault.rebase(); - // Matt received all the yield - await expect(matt).has.an.approxBalanceOf("300.00", ousd); - // Give contract 100 OUSD from Matt - await ousd.connect(matt).transfer(mockNonRebasing.address, ousdUnits("50")); - await expect(matt).has.an.approxBalanceOf("250", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); - // Transfer contract balance to Josh - await mockNonRebasing.increaseAllowance( - await matt.getAddress(), - ousdUnits("150") - ); + await ousd + .connect(matt) + .transferFrom( + mockNonRebasing.address, + await matt.getAddress(), + ousdUnits("100") + ); + await expect(matt).has.an.approxBalanceOf("200.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); - await ousd - .connect(matt) - .transferFrom( - mockNonRebasing.address, + it("non-rebasing account with previously set creditsPerToken to a rebasing account", async () => { + let { ousd, vault, matt, usdc, josh, mockNonRebasing } = fixture; + + // Give contract 100 OUSD from Josh + await ousd + .connect(josh) + .transfer(mockNonRebasing.address, ousdUnits("100")); + await expect(matt).has.an.approxBalanceOf("100.00", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("100.00", ousd); + // Transfer USDC into the Vault to simulate yield + await usdc.connect(matt).transfer(vault.address, usdcUnits("200")); + await vault.rebase(); + // Matt received all the yield + await expect(matt).has.an.approxBalanceOf("300.00", ousd); + // Give contract 100 OUSD from Matt + await ousd + .connect(matt) + .transfer(mockNonRebasing.address, ousdUnits("50")); + await expect(matt).has.an.approxBalanceOf("250", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("150.00", ousd); + // Transfer contract balance to Josh + await mockNonRebasing.increaseAllowance( await matt.getAddress(), ousdUnits("150") ); - await expect(matt).has.an.approxBalanceOf("400", ousd); - await expect(josh).has.an.approxBalanceOf("0", ousd); - await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + await ousd + .connect(matt) + .transferFrom( + mockNonRebasing.address, + await matt.getAddress(), + ousdUnits("150") + ); - // Validate rebasing and non rebasing credit accounting by calculating' - // total supply manually - const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) - .mul(utils.parseUnits("1", 18)) - .div(await ousd.rebasingCreditsPerTokenHighres()) - .add(await ousd.nonRebasingSupply()); - await expect(calculatedTotalSupply).to.approxEqual( - await ousd.totalSupply() - ); + await expect(matt).has.an.approxBalanceOf("400", ousd); + await expect(josh).has.an.approxBalanceOf("0", ousd); + await expect(mockNonRebasing).has.an.approxBalanceOf("0", ousd); + + // Validate rebasing and non rebasing credit accounting by calculating' + // total supply manually + const calculatedTotalSupply = (await ousd.rebasingCreditsHighres()) + .mul(utils.parseUnits("1", 18)) + .div(await ousd.rebasingCreditsPerTokenHighres()) + .add(await ousd.nonRebasingSupply()); + await expect(calculatedTotalSupply).to.approxEqual( + await ousd.totalSupply() + ); + }); }); it("Should maintain the correct balances when rebaseOptIn is called from non-rebasing contract", async () => { From bc710f31bc61cc6f6d9bfd6b0aba36f310009384 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 23 Oct 2024 20:30:10 +1100 Subject: [PATCH 5/6] Gas changes to changeSupply --- contracts/contracts/token/OUSD.sol | 51 +++++++++++++++++++----------- contracts/test/token/ousd.js | 13 ++++++++ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol index 6019a82cd7..56db3a4572 100644 --- a/contracts/contracts/token/OUSD.sol +++ b/contracts/contracts/token/OUSD.sol @@ -447,9 +447,11 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { returns (uint256) { // Read the account's non-rebasing credits per token from storage - uint256 creditsPerTokenMem = nonRebasingCreditsPerToken[_account]; - if (creditsPerTokenMem != 0) { - return creditsPerTokenMem; + uint256 nonRebasingCreditsPerTokenMem = nonRebasingCreditsPerToken[ + _account + ]; + if (nonRebasingCreditsPerTokenMem != 0) { + return nonRebasingCreditsPerTokenMem; } return _rebasingCreditsPerToken; @@ -582,35 +584,46 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { onlyVault nonReentrant { - require(_totalSupply > 0, "Cannot increase 0 supply"); + // Read values from storage into memory + uint256 totalSupplyMem = _totalSupply; + uint256 rebasingCreditsMem = _rebasingCredits; + uint256 rebasingCreditsPerTokenMem = _rebasingCreditsPerToken; + uint256 nonRebasingSupplyMem = nonRebasingSupply; - if (_totalSupply == _newTotalSupply) { + // Pre condition checks + require(totalSupplyMem > 0, "Cannot increase 0 supply"); + + if (totalSupplyMem == _newTotalSupply) { emit TotalSupplyUpdatedHighres( - _totalSupply, - _rebasingCredits, - _rebasingCreditsPerToken + totalSupplyMem, + rebasingCreditsMem, + rebasingCreditsPerTokenMem ); return; } - _totalSupply = _newTotalSupply > MAX_SUPPLY + // Calculate the new values in memory + totalSupplyMem = _newTotalSupply > MAX_SUPPLY ? MAX_SUPPLY : _newTotalSupply; - - _rebasingCreditsPerToken = _rebasingCredits.divPrecisely( - _totalSupply.sub(nonRebasingSupply) + rebasingCreditsPerTokenMem = rebasingCreditsMem.divPrecisely( + totalSupplyMem.sub(nonRebasingSupplyMem) ); + totalSupplyMem = rebasingCreditsMem + .divPrecisely(rebasingCreditsPerTokenMem) + .add(nonRebasingSupplyMem); - require(_rebasingCreditsPerToken > 0, "Invalid change in supply"); + // Post condition checks + require(rebasingCreditsPerTokenMem > 0, "Invalid change in supply"); - _totalSupply = _rebasingCredits - .divPrecisely(_rebasingCreditsPerToken) - .add(nonRebasingSupply); + // write the new values to storage + _totalSupply = totalSupplyMem; + _rebasingCreditsPerToken = rebasingCreditsPerTokenMem; emit TotalSupplyUpdatedHighres( - _totalSupply, - _rebasingCredits, - _rebasingCreditsPerToken + totalSupplyMem, + rebasingCreditsMem, + rebasingCreditsPerTokenMem ); } } diff --git a/contracts/test/token/ousd.js b/contracts/test/token/ousd.js index 707faf7383..3fab33d037 100644 --- a/contracts/test/token/ousd.js +++ b/contracts/test/token/ousd.js @@ -2,6 +2,7 @@ const { expect } = require("chai"); const { loadDefaultFixture } = require("../_fixture"); const { utils } = require("ethers"); +const { impersonateAndFund } = require("../../utils/signers.js"); const { daiUnits, ousdUnits, usdcUnits, isFork } = require("../helpers"); describe("Token", function () { @@ -56,6 +57,18 @@ describe("Token", function () { }); }); + it("Should measure gas of changeSupply", async () => { + const { ousd, vault } = fixture; + + const vaultSigner = await impersonateAndFund(vault.address); + const currentSupply = await ousd.totalSupply(); + + const tx = await ousd + .connect(vaultSigner) + .populateTransaction.changeSupply(currentSupply.add(ousdUnits("1"))); + await vaultSigner.sendTransaction(tx); + }); + it("Should return 0 balance for the zero address", async () => { const { ousd } = fixture; expect( From 5f54f832915070a216ca43dc1719f4374a7694e9 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Thu, 24 Oct 2024 12:27:09 +0200 Subject: [PATCH 6/6] add comment (#2288) --- contracts/contracts/token/OUSD.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol index 56db3a4572..7161a5c7b5 100644 --- a/contracts/contracts/token/OUSD.sol +++ b/contracts/contracts/token/OUSD.sol @@ -495,7 +495,12 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable { // high resolution, and do not have to do any other bookkeeping nonRebasingCreditsPerToken[_account] = 1e27; } else { - // Migrate an existing account: + // Migrate an existing account + // this can happen if an address sends tokens to an EOA address + // and then deploys the contract to that same address using create2 + // which allows for contract deploys to have predictable addresses. + // We don't need to update _creditBalances[_account] in this case since + // they already correspond to the global _rebasingCreditsPerToken. // Set fixed credits per token for this account nonRebasingCreditsPerToken[_account] = _rebasingCreditsPerToken;