From 507e18587b8a0b64a4bb21db01ecf876dc607e47 Mon Sep 17 00:00:00 2001 From: "hats-bug-reporter[bot]" <132391680+hats-bug-reporter[bot]@users.noreply.github.com> Date: Thu, 5 Sep 2024 14:54:30 +0000 Subject: [PATCH 1/8] Update audit competition README --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 7ef7387..3cfa598 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,11 @@ +# Audit Competition for Circles +This repository is for the audit competition for the Circles. +To participate, submit your findings only by using the on-chain submission process on https://app.hats.finance/vulnerability . +## How to participate +- follow the instructions on https://app.hats.finance/ +## Good luck! +We look forward to seeing your findings. +* * * # Circles Protocol Circles is a decentralized protocol for creating and distributing fair and social money through personal currencies. Built on the Gnosis Chain, it utilizes smart contracts to manage the creation, distribution, and transfer of personal currencies using the ERC1155 multi-token standard. From db93658e8d0b2cd9452002d39d9ba96c27061131 Mon Sep 17 00:00:00 2001 From: "hats-bug-reporter[bot]" <132391680+hats-bug-reporter[bot]@users.noreply.github.com> Date: Sun, 6 Oct 2024 13:00:43 +0000 Subject: [PATCH 2/8] Update report.md --- report.md | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 report.md diff --git a/report.md b/report.md new file mode 100644 index 0000000..eac2513 --- /dev/null +++ b/report.md @@ -0,0 +1,218 @@ +# **Circles Audit Competition on Hats.finance** + + +## Introduction to Hats.finance + + +Hats.finance builds autonomous security infrastructure for integration with major DeFi protocols to secure users' assets. +It aims to be the decentralized choice for Web3 security, offering proactive security mechanisms like decentralized audit competitions and bug bounties. +The protocol facilitates audit competitions to quickly secure smart contracts by having auditors compete, thereby reducing auditing costs and accelerating submissions. +This aligns with their mission of fostering a robust, secure, and scalable Web3 ecosystem through decentralized security solutions​. + +## About Hats Audit Competition + + +Hats Audit Competitions offer a unique and decentralized approach to enhancing the security of web3 projects. Leveraging the large collective expertise of hundreds of skilled auditors, these competitions foster a proactive bug hunting environment to fortify projects before their launch. Unlike traditional security assessments, Hats Audit Competitions operate on a time-based and results-driven model, ensuring that only successful auditors are rewarded for their contributions. This pay-for-results ethos not only allocates budgets more efficiently by paying exclusively for identified vulnerabilities but also retains funds if no issues are discovered. With a streamlined evaluation process, Hats prioritizes quality over quantity by rewarding the first submitter of a vulnerability, thus eliminating duplicate efforts and attracting top talent in web3 auditing. The process embodies Hats Finance's commitment to reducing fees, maintaining project control, and promoting high-quality security assessments, setting a new standard for decentralized security in the web3 space​​. + +## Circles Overview + +Circles is a decentralised web of trust-based social currency system issued on Gnosis Chain. + +## Competition Details + + +- Type: A public audit competition hosted by Circles +- Duration: 2 weeks +- Maximum Reward: $65,065 +- Submissions: 122 +- Total Payout: $23,026.5 distributed among 13 participants. + +## Scope of Audit + +## Project intro + +Circles is a decentralized protocol for creating and distributing fair and social money through personal and group currencies. Built on the Gnosis Chain, it utilizes smart contracts to manage the creation, distribution, and transfer of personal currencies using the ERC1155 multi-token standard. + + +## Audit competition scope + +`src/` directory of commit `rc-v0.3.6-alpha` in https://github.com/aboutcircles/circles-contracts-v2 + +``` +# File Structure + +├── names +│ ├── Base58Converter.sol +│ ├── INameRegistry.sol +│ └── NameRegistry.sol +├── proxy +│ ├── MasterCopyNonUpgradable.sol +│ ├── Proxy.sol +│ └── ProxyFactory.sol +├── operators +│ ├── BaseOperator.sol +│ └── SignedPathOperator.sol +├── circles +│ ├── BatchedDemurrage.sol +│ ├── Circles.sol +│ ├── Demurrage.sol +│ ├── DiscountedBalances.sol +│ ├── ERC1155.sol +│ ├── IDemurrage.sol +│ ├── ICircles.sol +│ └── InflationaryOperator.sol +├── groups +│ ├── BaseMintPolicy.sol +│ ├── Definitions.sol +│ └── IMintPolicy.sol +├── errors +│ └── Errors.sol +├── treasury +│ ├── IStandardVault.sol +│ ├── StandardTreasury.sol +│ └── StandardVault.sol +├── migration +│ ├── IHub.sol +│ ├── IToken.sol +│ └── Migration.sol +├── hub +│ ├── Hub.sol +│ ├── IHub.sol +│ └── TypeDefinitions.sol +└── lift + ├── DemurrageCircles.sol + ├── EIP712.sol + ├── ERC20DiscountedBalances.sol + ├── ERC20InflationaryBalances.sol + ├── ERC20Lift.sol + ├── ERC20Permit.sol + ├── IERC20Lift.sol + └── InflationaryCircles.sol + +``` + +## Medium severity issues + + +- **Reentrancy Vulnerability in personalMint() Allows Unlimited Token Claims** + + A vulnerability has been identified in the `personalMint()` function, which allows a registered user to repeatedly mint tokens for themselves through a reentrancy attack. This is due to the ability to reenter the `personalMint()` method via the `.onERC1155Received()` callback, allowing attackers to claim and mint tokens multiple times without restrictions. The core issue is the timing of the `mintTimes[_human].lastMintTime` update, leading to inaccurate issuance calculations upon reentry. This problem arises as the function does not prevent contracts from calling `registerHuman()` or `personalMint()`, thus enabling reentrancy. To address this, it is recommended to add a reentrancy guard, update `lastMintTime` before actual minting, and potentially restrict these functions to externally owned accounts (EOAs) only. This discovery is crucial for enhancing system security by preventing unlimited token issuance. + + + **Link**: [Issue #8](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/8) + + +- **Vulnerability in Stop Function Allows Resetting of Mint Status** + + The discussion centers around a flaw in the `stop` function, which is supposed to permanently prevent future mints by setting `lastMintTime` to `INDEFINITE_FUTURE`. However, it’s discovered that this can be reversed by calling `calculateIssuanceWithCheck`, which inadvertently resets `lastMintTime`, allowing mint operations to continue. This behavior isn't aligned with the protocol's intent for the `stop` function to be irreversible. Despite the issue allowing this "unstopping" of mints, it was assessed as low-priority since it doesn't pose a significant security risk in the current operational context. There's debate over whether this should be considered a medium-severity issue due to economic implications, but ultimately it's seen as a design oversight rather than a critical security flaw. + + + **Link**: [Issue #1](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/1) + +## Low severity issues + + +- **operateFlowMatrix allows potential disruptions through net-zero token flows** + + The operateFlowMatrix function allows manipulation of token balances with minimal approval, posing security risks. Attackers can disrupt or increase gas costs for token transfers with malicious reshufflings. Recommendations include tightening approval checks and exploring alternative security measures. Extended examples illustrate potential attacks, emphasizing the need for careful protocol design to prevent disruptions. + + + **Link**: [Issue #122](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/122) + + +- **Remove Hourly Check in _calculateIssuance to Improve Reward Claiming Efficiency** + + The `_calculateIssuance` function in a contract includes a check that blocks users from claiming rewards if less than an hour has passed since their last mint. This prevents users from receiving timely rewards. Removing this check could improve user experience without issues, as the function returns zero if no tokens are gained. + + + **Link**: [Issue #114](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/114) + + +- **Potential ShortName Collision Due to Lack of Zero-Value Check** + + A short name is generated using a keccak256 hash, restricted to about 70 bytes. If a calculated short name is 0, it breaks protocol invariants, causing potential confusion, as users may register duplicate short names. The solution is to add checks ensuring the short name is not 0. While the risk is low, implementing this check enhances code reliability. + + + **Link**: [Issue #113](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/113) + + +- **Incorrect StreamID Error in `operateFlowMatrix()` Function Due to Zero Indexing** + + In the `operateFlowMatrix()` function, there's a discrepancy between the error message and the actual stream index. The `streamid` parameter wrongly reflects the array index starting from 0, whereas it should start from 1. Adjusting the error message to include `i + 1` would correct this mismatch. While the code might remain unchanged, documentation updates could clarify this indexing for developers. + + + **Link**: [Issue #106](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/106) + + +- **Potential for Fake Token Mint Emissions in StandardTreasury Contract** + + The current setup allows anyone to send tokens to the treasury, potentially causing misleading mint events that could confuse event listeners. Suggested mitigations include moving the event to group mint or verifying the tokens' trustworthiness. Consideration is being given to renaming the event to "CollateralLocked" to avoid misinterpretation. + + + **Link**: [Issue #85](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/85) + + +- **Migration function allows zero-amount transactions, enabling unauthorized balance migration** + + The current `migrate` function implementation lacks validation to ensure the amount migrated is greater than zero. This loophole permits unauthorized zero-amount migrations, which could allow users to migrate others without holding tokens. The suggested fix is to add a check ensuring that the migration amount is greater than zero. + + + **Link**: [Issue #50](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/50) + + +- **Issue with Token Acceptance in Contract's Group Transfer Flow** + + The `_effectPathTransfers` function in the contract improperly forces a group, acting as a middle vertex in a token transfer flow, to accept tokens, even if not designated as a net receiver. This misalignment could cause failures if the group cannot hold tokens, disrupting intended transfer processes. It’s suggested the contract be adjusted to verify token acceptance only when necessary. + + + **Link**: [Issue #46](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/46) + + +- **Incorrect Error Revert Implementation in `_matchNettedFlows()` Function Causing Confusion** + + In the `_matchNettedFlows()` function of `Hub.sol`, an error is incorrectly implemented, leading to swapped values for `matrixNettedFlow` and `streamNettedFlow` in the `CirclesHubNettedFlowMismatch` revert statement. Although the functionality is unaffected, it may cause confusion during debugging. Correcting the error ensures better consistency and clarity. + + + **Link**: [Issue #41](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/41) + + +- **_incorrect hour calculation in _calculateIssuance function leads to inaccurate token issuance_** + + The `_calculateIssuance` function inaccurately calculates remaining hours when the current time exactly matches the hour. This miscalculation, due to an extra hour always being added, could lead to errors in token issuance and minting. Updating the logic to account for zeros in the remainder would resolve the issue efficiently. + + + **Link**: [Issue #39](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/39) + + +- **EnsureERC20 Function Lacks Proper Organization Check Allowing Unauthorized Access** + + The `wrap` function in the contract ensures only humans and groups can wrap tokens, excluding organizations. However, the `ensureERC20` function lacks this differentiation and only checks if the avatar is registered. To prevent bypassing restrictions, an additional check distinguishing organizations should be implemented in `ensureERC20`. + + + **Link**: [Issue #25](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/25) + + +- **Mismatch between documented and actual welcome bonus for new avatars in Circles** + + The documentation states that new avatars invited to Circles receive a welcome bonus of 50 Circles, but the actual bonus awarded is 48 Circles. This inconsistency between the documentation and the code could lead to confusion. The issue has been acknowledged and will be addressed in the upcoming update to ensure accurate documentation. + + + **Link**: [Issue #22](https://github.com/hats-finance/Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/issues/22) + + + +## Conclusion + +The Hats.finance audit competition for Circles, a decentralized social currency protocol on the Gnosis Chain, identified several vulnerabilities within the protocol's smart contracts. The competition, lasting two weeks, attracted 122 submissions and disbursed a total of $23,026.5 in rewards among 13 participants. Key issues identified included a reentrancy vulnerability that allowed users unlimited token minting through the `personalMint()` function, and a flaw in the `stop` function enabling the inadvertent continuation of mint operations. Both issues were deemed of medium severity. A range of low-severity issues was also noted, such as potential disruptions in token flows through minimal approvals, zero-amount migrations allowing unauthorized balance transfers, and inconsistencies in the welcome bonus documentation. Recommendations include implementing reentrancy guards, tightening approval checks, and updating documentation for clarity. These findings underscore the importance of thorough audits in enhancing decentralized protocol security and highlight Hats.finance's innovative approach to crowd-sourcing security efforts in the Web3 space. + +## Disclaimer + + +This report does not assert that the audited contracts are completely secure. Continuous review and comprehensive testing are advised before deploying critical smart contracts. + + +The Circles audit competition illustrates the collaborative effort in identifying and rectifying potential vulnerabilities, enhancing the overall security and functionality of the platform. + + +Hats.finance does not provide any guarantee or warranty regarding the security of this project. Smart contract software should be used at the sole risk and responsibility of users. + From bee59b1e330c5fbda625c4dbbb6592e7f8eff8ac Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Wed, 9 Oct 2024 17:32:09 +0100 Subject: [PATCH 3/8] (reviews): move Hats' report to reviews folder --- report.md => reviews/202409-hats/report.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename report.md => reviews/202409-hats/report.md (100%) diff --git a/report.md b/reviews/202409-hats/report.md similarity index 100% rename from report.md rename to reviews/202409-hats/report.md From 3c057eff6fcfc4cdbc3306e91668ee6459231de6 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Wed, 9 Oct 2024 17:33:28 +0100 Subject: [PATCH 4/8] (readme): remove Hats header on readme from their fork --- README.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/README.md b/README.md index 3cfa598..7ef7387 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,3 @@ -# Audit Competition for Circles -This repository is for the audit competition for the Circles. -To participate, submit your findings only by using the on-chain submission process on https://app.hats.finance/vulnerability . -## How to participate -- follow the instructions on https://app.hats.finance/ -## Good luck! -We look forward to seeing your findings. -* * * # Circles Protocol Circles is a decentralized protocol for creating and distributing fair and social money through personal currencies. Built on the Gnosis Chain, it utilizes smart contracts to manage the creation, distribution, and transfer of personal currencies using the ERC1155 multi-token standard. From a54a65d961389f0886ef13a0090505e7fb2f1196 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Wed, 9 Oct 2024 19:24:39 +0100 Subject: [PATCH 5/8] (deploy, hub): have deployment script compute bootstrap period duration to have it end 15 nov 2024; update hub to make param public --- script/deployments/chiadoDeploy.sh | 37 +++++++++++++++++++++++-- script/deployments/gnosisChainDeploy.sh | 37 +++++++++++++++++++++++-- script/deployments/package.json | 2 +- src/hub/Hub.sol | 2 +- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/script/deployments/chiadoDeploy.sh b/script/deployments/chiadoDeploy.sh index f33e1f2..052db33 100755 --- a/script/deployments/chiadoDeploy.sh +++ b/script/deployments/chiadoDeploy.sh @@ -74,8 +74,33 @@ V1_HUB_ADDRESS='0xdbF22D4e8962Db3b2F1d9Ff55be728A887e47710' # but like on mainnet we want to offset this to midnight to start day zero # on the Feb 1 2023, which has unix time 1675209600 INFLATION_DAY_ZERO=1675209600 -# put a long bootstrap time for testing bootstrap -BOOTSTRAP_ONE_YEAR=31540000 + +# Set the bootstrap end date to Nov 15, 2024 23:59:59 UTC +BOOTSTRAP_END_DATE="2024-11-15 23:59:59" + +# Function to convert UTC date to seconds since epoch +utc_to_seconds() { + if [[ "$OSTYPE" == "darwin"* ]]; then + # macOS + date -j -u -f "%Y-%m-%d %H:%M:%S" "$1" "+%s" + else + # GNU/Linux + date -u -d "$1" "+%s" + fi +} + +# Calculate the bootstrap period +CURRENT_TIME=$(date -u +"%Y-%m-%d %H:%M:%S") +BOOTSTRAP_END_SECONDS=$(utc_to_seconds "$BOOTSTRAP_END_DATE") +CURRENT_TIME_SECONDS=$(utc_to_seconds "$CURRENT_TIME") +BOOTSTRAP_PERIOD=$((BOOTSTRAP_END_SECONDS - CURRENT_TIME_SECONDS)) + +# Ensure the bootstrap period is not negative +if [ $BOOTSTRAP_PERIOD -lt 0 ]; then + echo "Error: The specified end date is in the past. Please update the BOOTSTRAP_END_DATE." >&2 + exit 1 +fi + # fallback URI URI='https://gateway.aboutcircles.com/v1/circles/{id}.json' @@ -133,7 +158,7 @@ HUB=$(deploy_and_store_details "Hub" $HUB_ADDRESS_01 \ --constructor-args $V1_HUB_ADDRESS \ $NAMEREGISTRY_ADDRESS_03 $MIGRATION_ADDRESS_02 $ERC20LIFT_ADDRESS_04 \ $STANDARD_TREASURY_ADDRESS_05 $INFLATION_DAY_ZERO \ - $BOOTSTRAP_ONE_YEAR $URI) + $BOOTSTRAP_PERIOD $URI) MIGRATION=$(deploy_and_store_details "Migration" $MIGRATION_ADDRESS_02 \ src/migration/Migration.sol:Migration \ @@ -190,4 +215,10 @@ summary_file="${OUT_DIR}/chiado-${identifier}.log" echo "MastercopyDemurrageERC20: ${MC_ERC20_DEMURRAGE}" echo "MastercopyInflationaryERC20: ${MC_ERC20_INFLATION}" echo "MastercopyStandardVault: ${MC_STANDARD_VAULT}" + echo "" + echo "Bootstrap End Date: $BOOTSTRAP_END_DATE UTC" + echo "Current Time: $CURRENT_TIME UTC" + echo "Bootstrap Period: ${BOOTSTRAP_PERIOD} seconds" + echo "Bootstrap End Date (Unix time): $BOOTSTRAP_END_SECONDS" + echo "Current Time (Unix time): $CURRENT_TIME_SECONDS" } >> "$summary_file" diff --git a/script/deployments/gnosisChainDeploy.sh b/script/deployments/gnosisChainDeploy.sh index c34f8e3..9824010 100755 --- a/script/deployments/gnosisChainDeploy.sh +++ b/script/deployments/gnosisChainDeploy.sh @@ -96,8 +96,33 @@ V1_HUB_ADDRESS='0x29b9a7fBb8995b2423a71cC17cf9810798F6C543' # but we want to offset this to midnight to start day zero # on midnight 15 October 2020 INFLATION_DAY_ZERO=1602720000 -# put a long bootstrap time for testing bootstrap to one year -BOOTSTRAP_ONE_YEAR=31540000 + +# Set the bootstrap end date to Nov 15, 2024 23:59:59 UTC +BOOTSTRAP_END_DATE="2024-11-15 23:59:59" + +# Function to convert UTC date to seconds since epoch +utc_to_seconds() { + if [[ "$OSTYPE" == "darwin"* ]]; then + # macOS + date -j -u -f "%Y-%m-%d %H:%M:%S" "$1" "+%s" + else + # GNU/Linux + date -u -d "$1" "+%s" + fi +} + +# Calculate the bootstrap period +CURRENT_TIME=$(date -u +"%Y-%m-%d %H:%M:%S") +BOOTSTRAP_END_SECONDS=$(utc_to_seconds "$BOOTSTRAP_END_DATE") +CURRENT_TIME_SECONDS=$(utc_to_seconds "$CURRENT_TIME") +BOOTSTRAP_PERIOD=$((BOOTSTRAP_END_SECONDS - CURRENT_TIME_SECONDS)) + +# Ensure the bootstrap period is not negative +if [ $BOOTSTRAP_PERIOD -lt 0 ]; then + echo "Error: The specified end date is in the past. Please update the BOOTSTRAP_END_DATE." >&2 + exit 1 +fi + # fallback URI URI='https://gateway.aboutcircles.com/v1/circles/{id}.json' @@ -160,7 +185,7 @@ HUB=$(deploy_and_store_details "Hub" $HUB_ADDRESS_01 \ --constructor-args $V1_HUB_ADDRESS \ $NAMEREGISTRY_ADDRESS_03 $MIGRATION_ADDRESS_02 $ERC20LIFT_ADDRESS_04 \ $STANDARD_TREASURY_ADDRESS_05 $INFLATION_DAY_ZERO \ - $BOOTSTRAP_ONE_YEAR $URI) + $BOOTSTRAP_PERIOD $URI) NONCE_USED=$((NONCE_USED + 1)) @@ -233,4 +258,10 @@ summary_file="${OUT_DIR}/gnosischain-${identifier}.log" echo "MastercopyDemurrageERC20: ${MC_ERC20_DEMURRAGE}" echo "MastercopyInflationaryERC20: ${MC_ERC20_INFLATION}" echo "MastercopyStandardVault: ${MC_STANDARD_VAULT}" + echo "" + echo "Bootstrap End Date: $BOOTSTRAP_END_DATE UTC" + echo "Current Time: $CURRENT_TIME UTC" + echo "Bootstrap Period: ${BOOTSTRAP_PERIOD} seconds" + echo "Bootstrap End Date (Unix time): $BOOTSTRAP_END_SECONDS" + echo "Current Time (Unix time): $CURRENT_TIME_SECONDS" } >> "$summary_file" diff --git a/script/deployments/package.json b/script/deployments/package.json index 40b01f0..088aaf6 100644 --- a/script/deployments/package.json +++ b/script/deployments/package.json @@ -1,6 +1,6 @@ { "name": "deploy-circles", - "version": "rc-0.3.7-alpha", + "version": "rc-0.3.7-beta", "type": "module", "dependencies": { "dotenv": "^16.4.5", diff --git a/src/hub/Hub.sol b/src/hub/Hub.sol index da4c524..aa48b40 100644 --- a/src/hub/Hub.sol +++ b/src/hub/Hub.sol @@ -73,7 +73,7 @@ contract Hub is Circles, TypeDefinitions, IHubErrors { * new avatars can be invited by registered avatars. After this time * only registered avatars can invite new avatars. */ - uint256 internal immutable invitationOnlyTime; + uint256 public immutable invitationOnlyTime; /** * @notice The standard treasury contract address used when From b21dcc2cd5dd147e8222ba9c66fd704fa294e62c Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Thu, 10 Oct 2024 18:45:15 +0100 Subject: [PATCH 6/8] (test): test inversion of gamma and beta exponentiation --- test/circles/Demurrage.t.sol | 122 ++++++++++++++++++++++++++++++++ test/circles/MockDemurrage.sol | 4 ++ test/setup/TimeCirclesSetup.sol | 2 +- 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/test/circles/Demurrage.t.sol b/test/circles/Demurrage.t.sol index 0586f75..15f0303 100644 --- a/test/circles/Demurrage.t.sol +++ b/test/circles/Demurrage.t.sol @@ -42,6 +42,8 @@ contract DemurrageTest is Test, TimeCirclesSetup, Approximation { startTime(); demurrage = new MockDemurrage(); + + demurrage.setInflationDayZero(INFLATION_DAY_ZERO); } // Tests @@ -55,4 +57,124 @@ contract DemurrageTest is Test, TimeCirclesSetup, Approximation { ); } } + + // Test the inversion accuracy of the gamma and beta exponentiation over 20 and 100 years + // with and without the extension + // conclusion: we can just drop the extension as the 64x64 fixed point is accurate, and unsure if the extension + // is actually doing something? -- maybe not because GAMMA and BETA have a fixed precision, so they will always + // introduce errors at their precision level... so the extension is pointless and costs extra gas + + // (note) leaving these tests here for now, to document why the extension is removed from Inflationary ERC20 in patch21! + // this can later be tidied up and removed + + function testInversionGammaBeta64x64_20years() public { + // for the coming 20 years (2024 is year 4 since INFLATION_DAY_ZERO) + // check that simply exponentiating the number of days remains accurate + // and without overflow + + // one year in unix time (approximately) + uint256 oneYear = 365 * 24 * 3600; + + for (uint256 i = 0; i <= 20; i++) { + uint256 secondsNow = INFLATION_DAY_ZERO + i * oneYear; + uint64 dayCount = demurrage.day(secondsNow); + // convert one CRC to inflationary value + uint256 inflationaryOneCRC = demurrage.convertDemurrageToInflationaryValue(100 * CRC, dayCount); + // now invert the operation + uint256 demurrageOneCRC = demurrage.convertInflationaryToDemurrageValue(inflationaryOneCRC, dayCount); + assertTrue(relativeApproximatelyEqual(100 * CRC, demurrageOneCRC, 1000 * DUST)); + console.log("year ", i, ": ", demurrageOneCRC); + } + } + + function testInversionGammaBeta64x64_100years() public { + // for the coming 100 years (2024 is year 4 since INFLATION_DAY_ZERO) + // check that simply exponentiating the number of days remains accurate + // and without overflow + + // one year in unix time (approximately) + uint256 oneYear = 365 * 24 * 3600; + + for (uint256 i = 0; i <= 100; i++) { + uint256 secondsNow = INFLATION_DAY_ZERO + i * oneYear; + uint64 dayCount = demurrage.day(secondsNow); + // convert one CRC to inflationary value + uint256 inflationaryOneCRC = demurrage.convertDemurrageToInflationaryValue(CRC, dayCount); + // now invert the operation + uint256 demurrageOneCRC = demurrage.convertInflationaryToDemurrageValue(inflationaryOneCRC, dayCount); + assertTrue(relativeApproximatelyEqual(CRC, demurrageOneCRC, 1000 * DUST)); + } + } + + function testInversionGammaBeta64x64_100years_withExtension() public { + // for the coming 100 years (2024 is year 4 since INFLATION_DAY_ZERO) + // check that simply exponentiating the number of days remains accurate + // and without overflow + + // one year in unix time (approximately) + uint256 oneYear = 365 * 24 * 3600; + + uint8 accuracy_shift = 64; + + uint192 amount = uint192(10000000 * CRC); + console.log("amount: ", amount); + + for (uint256 i = 0; i <= 100; i++) { + uint256 secondsNow = INFLATION_DAY_ZERO + i * oneYear; + uint64 dayCount = demurrage.day(secondsNow); + // convert one CRC to inflationary value + uint256 extendedAmount = amount << accuracy_shift; + uint256 inflationaryAmountExtended = demurrage.convertDemurrageToInflationaryValue(extendedAmount, dayCount); + uint256 trimmedInflationaryAmount = inflationaryAmountExtended >> accuracy_shift; + // now invert the operation + uint256 extendedInflationAmountTrimmed = trimmedInflationaryAmount << accuracy_shift; + uint256 demurrageAmountExtended = + demurrage.convertInflationaryToDemurrageValue(extendedInflationAmountTrimmed, dayCount); + uint256 trimmedDemurrageAmount = demurrageAmountExtended >> accuracy_shift; + assertTrue(relativeApproximatelyEqual(amount, trimmedDemurrageAmount, 1000 * DUST)); + console.log("year ", i, ": ", trimmedDemurrageAmount); + } + } + + function testInversionGammaBeta64x64_100years_withExtension_comparison() public { + // for the coming 100 years (2024 is year 4 since INFLATION_DAY_ZERO) + // check that simply exponentiating the number of days remains accurate + // and without overflow + + // one year in unix time (approximately) + uint256 oneYear = 365 * 24 * 3600; + + uint8 accuracy_shift = 64; + + uint192 amount = uint192(254516523121 * CRC); + console.log("amount: ", amount); + + for (uint256 i = 0; i <= 100; i++) { + uint256 secondsNow = INFLATION_DAY_ZERO + i * oneYear; + uint64 dayCount = demurrage.day(secondsNow); + // convert one CRC to inflationary value + uint256 extendedAmount = amount << accuracy_shift; + uint256 inflationaryAmountExtended = demurrage.convertDemurrageToInflationaryValue(extendedAmount, dayCount); + uint256 trimmedInflationaryAmount = inflationaryAmountExtended >> accuracy_shift; + // now invert the operation + uint256 extendedInflationAmountTrimmed = trimmedInflationaryAmount << accuracy_shift; + uint256 demurrageAmountExtended = + demurrage.convertInflationaryToDemurrageValue(extendedInflationAmountTrimmed, dayCount); + uint256 trimmedDemurrageAmount = demurrageAmountExtended >> accuracy_shift; + + // now do the same without extension + uint256 inflationaryAmount_withoutExtension = + demurrage.convertDemurrageToInflationaryValue(amount, dayCount); + // now invert the operation + uint256 demurrageAmount_withoutExtension = + demurrage.convertInflationaryToDemurrageValue(inflationaryAmount_withoutExtension, dayCount); + + uint256 diff = demurrageAmount_withoutExtension > trimmedDemurrageAmount + ? demurrageAmount_withoutExtension - trimmedDemurrageAmount + : trimmedDemurrageAmount - demurrageAmount_withoutExtension; + + assertTrue(diff == 0); + console.log(trimmedDemurrageAmount, " vs ", demurrageAmount_withoutExtension); + } + } } diff --git a/test/circles/MockDemurrage.sol b/test/circles/MockDemurrage.sol index 04faeb2..130f65e 100644 --- a/test/circles/MockDemurrage.sol +++ b/test/circles/MockDemurrage.sol @@ -14,6 +14,10 @@ contract MockDemurrage is Demurrage { return GAMMA_64x64; } + function beta_64x64() external pure returns (int128) { + return BETA_64x64; + } + function r(uint256 _i) external view returns (int128) { return R[_i]; } diff --git a/test/setup/TimeCirclesSetup.sol b/test/setup/TimeCirclesSetup.sol index e496974..350ed3f 100644 --- a/test/setup/TimeCirclesSetup.sol +++ b/test/setup/TimeCirclesSetup.sol @@ -7,7 +7,7 @@ import {StdCheats} from "forge-std/StdCheats.sol"; contract TimeCirclesSetup is Test { // Constants - uint256 internal constant CRC = uint256(10 ** 18); + uint192 internal constant CRC = uint192(10 ** 18); /** * Arbitrary origin for counting time since 10 December 2021 From 188cbb7fc11ad810b97e1d79ac6011efaf9417ba Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Thu, 10 Oct 2024 19:21:35 +0100 Subject: [PATCH 7/8] (InflationaryERC20): simplify conversion demurrage to inflationary, remove extension; clean up dead code --- src/circles/Demurrage.sol | 13 ------ src/circles/DiscountedBalances.sol | 10 ----- src/circles/InflationaryOperator.sol | 2 +- src/lift/DemurrageCircles.sol | 4 +- src/lift/ERC20DiscountedBalances.sol | 5 --- src/lift/ERC20InflationaryBalances.sol | 56 +++++++++----------------- src/lift/InflationaryCircles.sol | 5 +-- 7 files changed, 25 insertions(+), 70 deletions(-) diff --git a/src/circles/Demurrage.sol b/src/circles/Demurrage.sol index 3192ded..ae6526e 100644 --- a/src/circles/Demurrage.sol +++ b/src/circles/Demurrage.sol @@ -261,17 +261,4 @@ contract Demurrage is ICirclesCompactErrors, ICirclesDemurrageErrors { // and do not cache it return Math64x64.pow(GAMMA_64x64, _dayDifference); } - - /** - * Calculate the inflationary balance of a demurraged balance - * @param _balance Demurraged balance to calculate the inflationary balance of - * @param _dayUpdated The day the balance was last updated - */ - function _calculateInflationaryBalance(uint256 _balance, uint256 _dayUpdated) internal pure returns (uint256) { - // calculate the inflationary balance by dividing the balance by GAMMA^days - // note: GAMMA < 1, so dividing by a power of it, returns a bigger number, - // so the numerical imprecision is in the least significant bits. - int128 i = Math64x64.pow(BETA_64x64, _dayUpdated); - return Math64x64.mulu(i, _balance); - } } diff --git a/src/circles/DiscountedBalances.sol b/src/circles/DiscountedBalances.sol index 75ce835..e698608 100644 --- a/src/circles/DiscountedBalances.sol +++ b/src/circles/DiscountedBalances.sol @@ -78,16 +78,6 @@ contract DiscountedBalances is Demurrage { // Internal functions - /** - * @dev Calculate the inflationary balance of a discounted balance - * @param _account Address of the account to calculate the balance of - * @param _id Circles identifier for which to calculate the balance - */ - function _inflationaryBalanceOf(address _account, uint256 _id) internal view returns (uint256) { - DiscountedBalance memory discountedBalance = discountedBalances[_id][_account]; - return _calculateInflationaryBalance(discountedBalance.balance, discountedBalance.lastUpdatedDay); - } - /** * @dev Update the balance of an account for a given Circles identifier * @param _account Address of the account to update the balance of diff --git a/src/circles/InflationaryOperator.sol b/src/circles/InflationaryOperator.sol index 20f542b..36b8ded 100644 --- a/src/circles/InflationaryOperator.sol +++ b/src/circles/InflationaryOperator.sol @@ -94,6 +94,6 @@ contract InflationaryCirclesOperator is BatchedDemurrage { function _inflationaryBalanceOf(address _account, uint256 _id) internal view returns (uint256) { // retrieve the balance in demurrage units (of today) uint256 balance = hub.balanceOf(_account, _id); - return _calculateInflationaryBalance(balance, day(block.timestamp)); + return convertDemurrageToInflationaryValue(balance, day(block.timestamp)); } } diff --git a/src/lift/DemurrageCircles.sol b/src/lift/DemurrageCircles.sol index 5b674b3..1943a54 100644 --- a/src/lift/DemurrageCircles.sol +++ b/src/lift/DemurrageCircles.sol @@ -74,7 +74,7 @@ contract DemurrageCircles is MasterCopyNonUpgradable, ERC20DiscountedBalances, E _burn(msg.sender, _amount); hub.safeTransferFrom(address(this), msg.sender, toTokenId(avatar), _amount, ""); - uint256 inflationaryAmount = _calculateInflationaryBalance(_amount, day(block.timestamp)); + uint256 inflationaryAmount = convertDemurrageToInflationaryValue(_amount, day(block.timestamp)); emit WithdrawDemurraged(msg.sender, _amount, inflationaryAmount); } @@ -107,7 +107,7 @@ contract DemurrageCircles is MasterCopyNonUpgradable, ERC20DiscountedBalances, E if (_id != toTokenId(avatar)) revert CirclesInvalidCirclesId(_id, 0); _mint(_from, _amount); - uint256 inflationaryAmount = _calculateInflationaryBalance(_amount, day(block.timestamp)); + uint256 inflationaryAmount = convertDemurrageToInflationaryValue(_amount, day(block.timestamp)); emit DepositDemurraged(_from, _amount, inflationaryAmount); diff --git a/src/lift/ERC20DiscountedBalances.sol b/src/lift/ERC20DiscountedBalances.sol index 5b206e2..4cfffb5 100644 --- a/src/lift/ERC20DiscountedBalances.sol +++ b/src/lift/ERC20DiscountedBalances.sol @@ -102,11 +102,6 @@ contract ERC20DiscountedBalances is ERC20Permit, BatchedDemurrage, IERC20 { // Internal functions - function _inflationaryBalanceOf(address _account) internal view returns (uint256) { - DiscountedBalance memory discountedBalance = discountedBalances[_account]; - return _calculateInflationaryBalance(discountedBalance.balance, discountedBalance.lastUpdatedDay); - } - function _updateBalance(address _account, uint256 _balance, uint64 _day) internal { if (_balance > MAX_VALUE) { // Balance exceeds maximum value. diff --git a/src/lift/ERC20InflationaryBalances.sol b/src/lift/ERC20InflationaryBalances.sol index bb04e92..9cff8fc 100644 --- a/src/lift/ERC20InflationaryBalances.sol +++ b/src/lift/ERC20InflationaryBalances.sol @@ -6,15 +6,11 @@ import "../circles/BatchedDemurrage.sol"; import "./ERC20Permit.sol"; contract ERC20InflationaryBalances is ERC20Permit, BatchedDemurrage, IERC20 { - // Constants - - uint8 internal constant EXTENDED_ACCURACY_BITS = 64; - // State variables - uint256 internal _extendedTotalSupply; + uint256 internal _totalSupply; - mapping(address => uint256) private _extendedAccuracyBalances; + mapping(address => uint256) private _balances; // Constructor @@ -55,7 +51,7 @@ contract ERC20InflationaryBalances is ERC20Permit, BatchedDemurrage, IERC20 { } function balanceOf(address _account) external view returns (uint256) { - return _extendedAccuracyBalances[_account] >> EXTENDED_ACCURACY_BITS; + return _balances[_account]; } function allowance(address _owner, address _spender) external view returns (uint256) { @@ -63,58 +59,46 @@ contract ERC20InflationaryBalances is ERC20Permit, BatchedDemurrage, IERC20 { } function totalSupply() external view returns (uint256) { - return _extendedTotalSupply >> EXTENDED_ACCURACY_BITS; + return _totalSupply; } // Internal functions - function _convertToExtended(uint256 _amount) internal pure returns (uint256) { - if (_amount > MAX_VALUE) revert CirclesAmountOverflow(_amount, 0); - return _amount << EXTENDED_ACCURACY_BITS; - } - function _transfer(address _from, address _to, uint256 _amount) internal { - uint256 extendedAmount = _convertToExtended(_amount); - uint256 extendedFromBalance = _extendedAccuracyBalances[_from]; - if (extendedFromBalance < extendedAmount) { - revert ERC20InsufficientBalance(_from, extendedFromBalance >> EXTENDED_ACCURACY_BITS, _amount); + uint256 fromBalance = _balances[_from]; + if (fromBalance < _amount) { + revert ERC20InsufficientBalance(_from, fromBalance, _amount); } unchecked { - _extendedAccuracyBalances[_from] = extendedFromBalance - extendedAmount; + _balances[_from] = fromBalance - _amount; // rely on total supply not having overflowed - _extendedAccuracyBalances[_to] += extendedAmount; + _balances[_to] += _amount; } emit Transfer(_from, _to, _amount); } function _mintFromDemurragedAmount(address _owner, uint256 _demurragedAmount) internal returns (uint256) { - // first convert to extended accuracy representation so we have extra garbage bits, - // before we apply the inflation factor, which will produce errors in the least significant bits - uint256 extendedAmount = - _calculateInflationaryBalance(_convertToExtended(_demurragedAmount), day(block.timestamp)); + uint256 inflationaryAmount = convertDemurrageToInflationaryValue(_demurragedAmount, day(block.timestamp)); // here ensure total supply does not overflow - _extendedTotalSupply += extendedAmount; + _totalSupply += inflationaryAmount; unchecked { - _extendedAccuracyBalances[_owner] += extendedAmount; + _balances[_owner] += inflationaryAmount; } - emit Transfer(address(0), _owner, extendedAmount >> EXTENDED_ACCURACY_BITS); + emit Transfer(address(0), _owner, inflationaryAmount); - return extendedAmount >> EXTENDED_ACCURACY_BITS; + return inflationaryAmount; } - function _burn(address _owner, uint256 _amount) internal returns (uint256) { - uint256 extendedAmount = _convertToExtended(_amount); - uint256 extendedOwnerBalance = _extendedAccuracyBalances[_owner]; - if (extendedOwnerBalance < extendedAmount) { - revert ERC20InsufficientBalance(_owner, _extendedAccuracyBalances[_owner], _amount); + function _burn(address _owner, uint256 _amount) internal { + uint256 ownerBalance = _balances[_owner]; + if (ownerBalance < _amount) { + revert ERC20InsufficientBalance(_owner, ownerBalance, _amount); } unchecked { - _extendedAccuracyBalances[_owner] = extendedOwnerBalance - extendedAmount; + _balances[_owner] = ownerBalance - _amount; // rely on total supply tracking complete sum of balances - _extendedTotalSupply -= extendedAmount; + _totalSupply -= _amount; } emit Transfer(_owner, address(0), _amount); - - return extendedAmount; } } diff --git a/src/lift/InflationaryCircles.sol b/src/lift/InflationaryCircles.sol index b488e71..bdd4d83 100644 --- a/src/lift/InflationaryCircles.sol +++ b/src/lift/InflationaryCircles.sol @@ -78,11 +78,10 @@ contract InflationaryCircles is MasterCopyNonUpgradable, ERC20InflationaryBalanc // External functions function unwrap(uint256 _amount) external { - uint256 extendedAmount = _burn(msg.sender, _amount); + _burn(msg.sender, _amount); // calculate demurraged amount in extended accuracy representation // then discard garbage bits by shifting right - uint256 demurragedAmount = - convertInflationaryToDemurrageValue(extendedAmount, day(block.timestamp)) >> EXTENDED_ACCURACY_BITS; + uint256 demurragedAmount = convertInflationaryToDemurrageValue(_amount, day(block.timestamp)); hub.safeTransferFrom(address(this), msg.sender, toTokenId(avatar), demurragedAmount, ""); From 6701cf1dda8953f79e57f2dcf15dac84f592bf54 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 11 Oct 2024 17:36:50 +0100 Subject: [PATCH 8/8] (test/erc20Inflationary): write simple test to assert unwrap erc20 inflationary also converts back to demurrage --- ...RC20Demurrage.sol => ERC20Demurrage.t.sol} | 0 test/lift/ERC20Inflationary.t.sol | 92 +++++++++++++++++++ 2 files changed, 92 insertions(+) rename test/lift/{ERC20Demurrage.sol => ERC20Demurrage.t.sol} (100%) create mode 100644 test/lift/ERC20Inflationary.t.sol diff --git a/test/lift/ERC20Demurrage.sol b/test/lift/ERC20Demurrage.t.sol similarity index 100% rename from test/lift/ERC20Demurrage.sol rename to test/lift/ERC20Demurrage.t.sol diff --git a/test/lift/ERC20Inflationary.t.sol b/test/lift/ERC20Inflationary.t.sol new file mode 100644 index 0000000..7705ee2 --- /dev/null +++ b/test/lift/ERC20Inflationary.t.sol @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.13; + +import {Test} from "forge-std/Test.sol"; +import {StdCheats} from "forge-std/StdCheats.sol"; +import "forge-std/console.sol"; +import "../../src/circles/Demurrage.sol"; +import "../setup/TimeCirclesSetup.sol"; +import "../setup/HumanRegistration.sol"; +import "../hub/MockDeployment.sol"; +import "../hub/MockHub.sol"; +import "../utils/Approximation.sol"; + +contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration, Approximation { + // State variables + + MockDeployment public mockDeployment; + MockHub public hub; + + mapping(address => InflationaryCircles) public erc20s; + + // Constructor + + constructor() HumanRegistration(2) {} + + // Setup + + function setUp() public { + // Set time in 2021 + startTime(); + + // Mock deployment + mockDeployment = new MockDeployment(INFLATION_DAY_ZERO, 365 days); + hub = mockDeployment.hub(); + + // register Alice and Bob + for (uint256 i = 0; i < 2; i++) { + vm.startPrank(addresses[i]); + hub.registerHumanUnrestricted(); + mockDeployment.nameRegistry().registerShortName(); + vm.stopPrank(); + } + + // skip time and mint + skipTime(14 days); + for (uint256 i = 0; i < 2; i++) { + vm.prank(addresses[i]); + hub.personalMintWithoutV1Check(); + } + + // lift some CRC into respective inflationary ERC20's + for (uint256 i = 0; i < 2; i++) { + vm.prank(addresses[i]); + hub.wrap(addresses[i], 100 * CRC, CirclesType.Inflation); + } + + // get ERC20's + for (uint256 i = 0; i < 2; i++) { + erc20s[addresses[i]] = + InflationaryCircles(mockDeployment.erc20Lift().erc20Circles(CirclesType.Inflation, addresses[i])); + } + } + + // Tests + + function testWrapAndUnwrapInflationaryERC20() public { + InflationaryCircles aliceERC20 = erc20s[addresses[0]]; + uint256 aliceId = hub.toTokenId(addresses[0]); + + // Alice first clears her balance on the hub + uint256 aliceBalance = hub.balanceOf(addresses[0], aliceId); + console.log("Alice balance on hub: ", aliceBalance); + vm.prank(addresses[0]); + hub.burn(aliceId, aliceBalance, ""); + aliceBalance = hub.balanceOf(addresses[0], aliceId); + console.log("Alice balance on hub after burn: ", aliceBalance); + + // Alice unwraps her 100 CRC (demurrage) + // first get her balance in inflationary ERC20 + uint256 aliceERC20Balance = aliceERC20.balanceOf(addresses[0]); + console.log("Alice balance in inflationary ERC20: ", aliceERC20Balance); + vm.prank(addresses[0]); + aliceERC20.unwrap(aliceERC20Balance); + aliceERC20Balance = aliceERC20.balanceOf(addresses[0]); + console.log("Alice balance in inflationary ERC20 after unwrap: ", aliceERC20Balance); + + // Alice balance on hub should be 100 CRC again + aliceBalance = hub.balanceOf(addresses[0], aliceId); + console.log("Alice balance on hub after unwrap: ", aliceBalance); + assertTrue(relativeApproximatelyEqual(aliceBalance, 100 * CRC, 10 * DUST)); + } +}