Skip to content

Commit

Permalink
Change rounding descriptions
Browse files Browse the repository at this point in the history
  • Loading branch information
ernestognw committed Jul 13, 2023
1 parent 801ca61 commit 938bce0
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 11 deletions.
2 changes: 1 addition & 1 deletion contracts/utils/Arrays.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ library Arrays {
uint256 mid = Math.average(low, high);

// Note that mid will always be strictly less than high (i.e. it will be a valid array index)
// because Math.average rounds down (it does integer division with truncation).
// because Math.average rounds towards zero (it does integer division with truncation).
if (unsafeAccess(array, mid).value > element) {
high = mid;
} else {
Expand Down
13 changes: 7 additions & 6 deletions contracts/utils/math/Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ library Math {
/**
* @dev Returns the ceiling of the division of two numbers.
*
* This differs from standard division with `/` in that it rounds up instead
* of rounding down.
* This differs from standard division with `/` in that it rounds towards infinity instead
* of rounding towards zero.
*/
function ceilDiv(uint256 a, uint256 b) internal pure returns (uint256) {
if (b == 0) {
Expand Down Expand Up @@ -214,7 +214,8 @@ library Math {
}

/**
* @dev Returns the square root of a number. If the number is not a perfect square, the value is rounded down.
* @dev Returns the square root of a number. If the number is not a perfect square, the value is rounded
* towards zero.
*
* Inspired by Henry S. Warren, Jr.'s "Hacker's Delight" (Chapter 11).
*/
Expand Down Expand Up @@ -262,7 +263,7 @@ library Math {
}

/**
* @dev Return the log in base 2, rounded down, of a positive value.
* @dev Return the log in base 2 of a positive value rounded towards zero.
* Returns 0 if given 0.
*/
function log2(uint256 value) internal pure returns (uint256) {
Expand Down Expand Up @@ -315,7 +316,7 @@ library Math {
}

/**
* @dev Return the log in base 10, rounded down, of a positive value.
* @dev Return the log in base 10 of a positive value rounded towards zero.
* Returns 0 if given 0.
*/
function log10(uint256 value) internal pure returns (uint256) {
Expand Down Expand Up @@ -364,7 +365,7 @@ library Math {
}

/**
* @dev Return the log in base 256, rounded down, of a positive value.
* @dev Return the log in base 256 of a positive value rounded towards zero.
* Returns 0 if given 0.
*
* Adding one to the result gives the number of pairs of hex symbols needed to represent `value` as a hex string.
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/erc4626.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ image::erc4626-rate-loglogext.png[More exchange rates in logarithmic scale]

=== The attack

When depositing tokens, the number of shares a user gets is rounded down. This rounding takes away value from the user in favor or the vault (i.e. in favor of all the current share holders). This rounding is often negligible because of the amount at stake. If you deposit 1e9 shares worth of tokens, the rounding will have you lose at most 0.0000001% of your deposit. However if you deposit 10 shares worth of tokens, you could lose 10% of your deposit. Even worse, if you deposit <1 share worth of tokens, then you get 0 shares, and you basically made a donation.
When depositing tokens, the number of shares a user gets is rounded towards zero. This rounding takes away value from the user in favor or the vault (i.e. in favor of all the current share holders). This rounding is often negligible because of the amount at stake. If you deposit 1e9 shares worth of tokens, the rounding will have you lose at most 0.0000001% of your deposit. However if you deposit 10 shares worth of tokens, you could lose 10% of your deposit. Even worse, if you deposit <1 share worth of tokens, then you get 0 shares, and you basically made a donation.

For a given amount of assets, the more shares you receive the safer you are. If you want to limit your losses to at most 1%, you need to receive at least 100 shares.

Expand Down
6 changes: 3 additions & 3 deletions test/token/ERC20/extensions/ERC4626.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,8 @@ contract('ERC4626', function (accounts) {
}

// 5. Bob mints 2000 shares (costs 3001 assets)
// NOTE: Bob's assets spent got rounded up
// NOTE: Alices's vault assets got rounded up
// NOTE: Bob's assets spent got rounded towards infinity
// NOTE: Alices's vault assets got rounded towards infinity
{
const { tx } = await this.vault.mint(2000, user2, { from: user2 });
await expectEvent.inTransaction(tx, this.token, 'Transfer', {
Expand Down Expand Up @@ -1056,7 +1056,7 @@ contract('ERC4626', function (accounts) {
}

// 9. Alice withdraws 3643 assets (2000 shares)
// NOTE: Bob's assets have been rounded back up
// NOTE: Bob's assets have been rounded back towards infinity
{
const { tx } = await this.vault.withdraw(3643, user1, user1, { from: user1 });
await expectEvent.inTransaction(tx, this.vault, 'Transfer', {
Expand Down

0 comments on commit 938bce0

Please sign in to comment.