Giter Club home page Giter Club logo

properties's Introduction

Table of contents

Properties

This repository contains 168 code properties for:

The goals of these properties are to:

  • Detect vulnerabilities
  • Ensure adherence to relevant standards
  • Provide educational guidance for writing invariants

The properties can be used through unit tests or through fuzzing with Echidna or Medusa.

Testing the properties with fuzzing

  1. Install Echidna or Medusa.

  2. Import the properties into to your project:

    • In case of using Hardhat, use: npm install https://github.com/crytic/properties.git or yarn add https://github.com/crytic/properties.git
    • In case of using Foundry, use: forge install crytic/properties
  3. According to tests required, go the the specific sections:

ERC20 tests

To test an ERC20 token, follow these steps:

  1. Integration
  2. Configuration
  3. Run

You can see the output for a compliant token, and non compliant token.

Integration

Decide if you want to do internal or external testing. Both approaches have advantages and disadvantages, you can check more information about them here.

For internal testing, create a new Solidity file containing the CryticERC20InternalHarness contract. USER1, USER2 and USER3 constants are initialized by default in PropertiesConstants contract to be the addresses from where echidna sends transactions, and INITIAL_BALANCE is by default 1000e18:

pragma solidity ^0.8.0;
import "@crytic/properties/contracts/ERC20/internal/properties/ERC20BasicProperties.sol";
import "./MyToken.sol";
contract CryticERC20InternalHarness is MyToken, CryticERC20BasicProperties {

    constructor() {
        // Setup balances for USER1, USER2 and USER3:
        _mint(USER1, INITIAL_BALANCE);
        _mint(USER2, INITIAL_BALANCE);
        _mint(USER3, INITIAL_BALANCE);
        // Setup total supply:
        initialSupply = totalSupply();
    }
}

For external testing, create two contracts: the CryticERC20ExternalHarness and the TokenMock as shown below. In the CryticERC20ExternalHarness contract you can specify which properties to test, via inheritance. In the TokenMock contract, you will need to modify the isMintableOrBurnable variable if your contract is able to mint or burn tokens.

pragma solidity ^0.8.0;
import "./MyToken.sol";
import {ITokenMock} from "@crytic/properties/contracts/ERC20/external/util/ITokenMock.sol";
import {CryticERC20ExternalBasicProperties} from "@crytic/properties/contracts/ERC20/external/properties/ERC20ExternalBasicProperties.sol";
import {PropertiesConstants} from "@crytic/properties/contracts/util/PropertiesConstants.sol";


contract CryticERC20ExternalHarness is CryticERC20ExternalBasicProperties {
    constructor() {
        // Deploy ERC20
        token = ITokenMock(address(new CryticTokenMock()));
    }
}

contract CryticTokenMock is MyToken, PropertiesConstants {

    bool public isMintableOrBurnable;
    uint256 public initialSupply;
    constructor () {
        _mint(USER1, INITIAL_BALANCE);
        _mint(USER2, INITIAL_BALANCE);
        _mint(USER3, INITIAL_BALANCE);
        _mint(msg.sender, INITIAL_BALANCE);

        initialSupply = totalSupply();
        isMintableOrBurnable = true;
    }
}

Configuration

Echidna

Create the following Echidna config file

corpusDir: "tests/crytic/erc20/echidna-corpus-internal"
testMode: assertion
testLimit: 100000
deployer: "0x10000"
sender: ["0x10000", "0x20000", "0x30000"]
# Uncomment the following line for external testing
#allContracts: true

Medusa

Create the following Medusa config file:

{
	"fuzzing": {
    "testLimit": 100000,
		"corpusDirectory": "tests/medusa-corpus",
		"deployerAddress": "0x10000",
		"senderAddresses": [
			"0x10000",
			"0x20000",
			"0x30000"
		],
		"assertionTesting": {
			"enabled": true
		},
		"propertyTesting": {
			"enabled": false
		},
    "optimizationTesting": {
				"enabled": false,
		},
	},
// Uncomment the following lines for external testing
//		"testing": {
//			"testAllContracts": true
//    },
	"compilation": {
		"platform": "crytic-compile",
		"platformConfig": {
			"target": ".",
			"solcVersion": "",
			"exportDirectory": "",
			"args": ["--foundry-compile-all"]
		}
	}
}

To perform more than one test, save the files with a descriptive path, to identify what test each file or corpus belongs to. For instace, for these examples, we use tests/crytic/erc20/echidna-internal.yaml and tests/crytic/erc20/echidna-external.yaml for the Echidna tests for ERC20. We recommended to modify the corpus directory config opction for external tests accordingly.

The above configuration will start Echidna or Medusa in assertion mode. The target contract(s) will be deployed from address 0x10000, and transactions will be sent from the owner as well as two different users (0x20000 and 0x30000). There is an initial limit of 100000 tests, but depending on the token code complexity, this can be increased. Finally, once our fuzzing tools finish the fuzzing campaign, corpus and coverage results will be available in the specified corpus directory.

Run

Echidna

  • For internal testing: echidna . --contract CryticERC20InternalHarness --config tests/crytic/erc20/echidna-internal.yaml
  • For external testing: echidna . --contract CryticERC20ExternalHarness --config tests/crytic/erc20/echidna-external.yaml

Medusa

  • Go to the directory cd tests/crytic/erc20
  • For internal testing: medusa fuzz --target-contracts CryticERC20InternalHarness --config medusa-internal.yaml
  • For external testing: medusa fuzz --target-contracts CryticERC20ExternalHarness --config medusa-external.yaml

Example: Output for a compliant token

If the token under test is compliant and no properties will fail during fuzzing, the Echidna output should be similar to the screen below:

$ echidna . --contract CryticERC20InternalHarness --config tests/echidna.config.yaml
Loaded total of 23 transactions from corpus/coverage
Analyzing contract: contracts/ERC20/CryticERC20InternalHarness.sol:CryticERC20InternalHarness
name():  passed! ๐ŸŽ‰
test_ERC20_transferFromAndBurn():  passed! ๐ŸŽ‰
approve(address,uint256):  passed! ๐ŸŽ‰
test_ERC20_userBalanceNotHigherThanSupply():  passed! ๐ŸŽ‰
totalSupply():  passed! ๐ŸŽ‰
...

Example: Output for a non-compliant token

For this example, the ExampleToken's approval function was modified to perform no action:

function approve(address spender, uint256 amount) public virtual override(ERC20) returns (bool) {
  // do nothing
  return true;
}

In this case, the Echidna output should be similar to the screen below, notice that all functions that rely on approve() to work correctly will have their assertions broken, and will report the situation.

$ echidna . --contract CryticERC20ExternalHarness --config tests/echidna.config.yaml
Loaded total of 25 transactions from corpus/coverage
Analyzing contract: contracts/ERC20/CryticERC20ExternalHarness.sol:CryticERC20ExternalHarness
name():  passed! ๐ŸŽ‰
test_ERC20_transferFromAndBurn():  passed! ๐ŸŽ‰
approve(address,uint256):  passed! ๐ŸŽ‰
...
test_ERC20_setAllowance(): failed!๐Ÿ’ฅ
  Call sequence:
    test_ERC20_setAllowance()

Event sequence: Panic(1), AssertEqFail("Equal assertion failed. Message: Failed to set allowance") from: ERC20PropertyTests@0x00a329c0648769A73afAc7F9381E08FB43dBEA72
...

ERC721 tests

To test an ERC721 token, follow these steps:

  1. Integration
  2. Configuration
  3. Run

You can see the output for a compliant token, and non compliant token.

Integration

Decide if you want to do internal or external testing. Both approaches have advantages and disadvantages, you can check more information about them here.

For internal testing, create a new Solidity file containing the CryticERC721InternalHarness contract. USER1, USER2 and USER3 constants are initialized by default in PropertiesConstants contract to be the addresses from where echidna sends transactions.

pragma solidity ^0.8.0;
import "@crytic/properties/contracts/ERC721/internal/properties/ERC721BasicProperties.sol";
import "./MyToken.sol";
contract CryticERC721InternalHarness is MyToken, CryticERC721BasicProperties {

    constructor() {
    }
}

For external testing, create two contracts: the CryticERC721ExternalHarness and the TokenMock as shown below. In the CryticERC721ExternalHarness contract you can specify which properties to test, via inheritance. In the TokenMock contract, you will need to modify the isMintableOrBurnable variable if your contract is able to mint or burn tokens.

pragma solidity ^0.8.0;
import "./MyToken.sol";
import {ITokenMock} from "@crytic/properties/contracts/ERC721/external/util/ITokenMock.sol";
import {CryticERC721ExternalBasicProperties} from "@crytic/properties/contracts/ERC721/external/properties/ERC721ExternalBasicProperties.sol";
import {PropertiesConstants} from "@crytic/properties/contracts/util/PropertiesConstants.sol";


contract CryticERC721ExternalHarness is CryticERC721ExternalBasicProperties {   
    constructor() {
        // Deploy ERC721
        token = ITokenMock(address(new CryticTokenMock()));
    }
}

contract CryticTokenMock is MyToken, PropertiesConstants {

    bool public isMintableOrBurnable;

    constructor () {
        isMintableOrBurnable = true;
    }
}

Configuration

Create the following Echidna config file

corpusDir: "tests/crytic/erc721/echidna-corpus-internal"
testMode: assertion
testLimit: 100000
deployer: "0x10000"
sender: ["0x10000", "0x20000", "0x30000"]

If you're using external testing, you will also need to specify:

allContracts: true

To perform more than one test, save the files with a descriptive path, to identify what test each file or corpus belongs to. For these examples, we use tests/crytic/erc721/echidna-internal.yaml and tests/crytic/erc721/echidna-external.yaml for the Echidna tests for ERC721. We recommended to modify the corpusDir for external tests accordingly.

The above configuration will start Echidna in assertion mode. Contract will be deployed from address 0x10000, and transactions will be sent from the owner and two different users (0x20000 and 0x30000). There is an initial limit of 100000 tests, but depending on the token code complexity, this can be increased. Finally, once Echidna finishes the fuzzing campaign, corpus and coverage results will be available in the tests/crytic/erc721/echidna-corpus-internal directory.

Run

Run Echidna:

  • For internal testing: echidna . --contract CryticERC721InternalHarness --config tests/crytic/erc721/echidna-internal.yaml
  • For external testing: echidna . --contract CryticERC721ExternalHarness --config tests/crytic/erc721/echidna-external.yaml

Finally, inspect the coverage report in tests/crytic/erc721/echidna-corpus-internal or tests/crytic/erc721/echidna-corpus-external when it finishes.

Example: Output for a compliant token

If the token under test is compliant and no properties will fail during fuzzing, the Echidna output should be similar to the screen below:

$ echidna . --contract CryticERC721InternalHarness --config tests/echidna.config.yaml 
Loaded total of 23 transactions from corpus/coverage
Analyzing contract: contracts/ERC721/CryticERC721InternalHarness.sol:CryticERC721InternalHarness
name():  passed! ๐ŸŽ‰
test_ERC721_external_transferFromNotApproved():  passed! ๐ŸŽ‰
approve(address,uint256):  passed! ๐ŸŽ‰
test_ERC721_external_transferFromUpdatesOwner():  passed! ๐ŸŽ‰
totalSupply():  passed! ๐ŸŽ‰
...

Example: Output for a non-compliant token

For this example, the ExampleToken's balanceOf function was modified so it does not check that owner is the zero address:

function balanceOf(address owner) public view virtual override returns (uint256) {
    return _balances[owner];
}

In this case, the Echidna output should be similar to the screen below, notice that all functions that rely on balanceOf() to work correctly will have their assertions broken, and will report the situation.

$ echidna . --contract CryticERC721ExternalHarness --config tests/echidna.config.yaml
Loaded total of 25 transactions from corpus/coverage
Analyzing contract: contracts/ERC721/CryticERC721ExternalHarness.sol:CryticERC721ExternalHarness
name():  passed! ๐ŸŽ‰
test_ERC721_external_transferFromUpdatesOwner():  passed! ๐ŸŽ‰
approve(address,uint256):  passed! ๐ŸŽ‰
...
test_ERC721_external_balanceOfZeroAddressMustRevert(): failed!๐Ÿ’ฅ  
  Call sequence:
    test_ERC721_external_balanceOfZeroAddressMustRevert()

Event sequence: Panic(1), AssertFail("address(0) balance query should have reverted") from: ERC721PropertyTests@0x00a329c0648769A73afAc7F9381E08FB43dBEA72
...

ERC4626 Tests

To test an ERC4626 token, follow these steps:

  1. Integration
  2. Configuration
  3. Run

Integration

Create a new Solidity file containing the CryticERC4626Harness contract. Make sure it properly initializes your ERC4626 vault with a test token (TestERC20Token):

If you are using Hardhat:

    import {CryticERC4626PropertyTests} from "@crytic/properties/contracts/ERC4626/ERC4626PropertyTests.sol";
    // this token _must_ be the vault's underlying asset
    import {TestERC20Token} from "@crytic/properties/contracts/ERC4626/util/TestERC20Token.sol";
    // change to your vault implementation
    import "./Basic4626Impl.sol";

    contract CryticERC4626Harness is CryticERC4626PropertyTests {
        constructor () {
            TestERC20Token _asset = new TestERC20Token("Test Token", "TT", 18);
            Basic4626Impl _vault = new Basic4626Impl(_asset);
            initialize(address(_vault), address(_asset), false);
        }
    }

If you are using Foundry:

    import {CryticERC4626PropertyTests} from "properties/ERC4626/ERC4626PropertyTests.sol";
    // this token _must_ be the vault's underlying asset
    import {TestERC20Token} from "properties/ERC4626/util/TestERC20Token.sol";
    // change to your vault implementation
    import "../src/Basic4626Impl.sol";

    contract CryticERC4626Harness is CryticERC4626PropertyTests {
        constructor () {
            TestERC20Token _asset = new TestERC20Token("Test Token", "TT", 18);
            Basic4626Impl _vault = new Basic4626Impl(_asset);
            initialize(address(_vault), address(_asset), false);
        }
    }

Configuration

Create a minimal Echidna config file (e.g. tests/echidna.config.yaml)

corpusDir: "tests/echidna-corpus"
testMode: assertion
testLimit: 100000
deployer: "0x10000"
sender: ["0x10000"]

Run

Run the test suite using echidna . --contract CryticERC4626Harness --config tests/echidna.config.yaml and inspect the coverage report in tests/echidna-corpus when it finishes.

Example repositories are available for Hardhat and Foundry.

Once things are up and running, consider adding internal testing methods to your Vault ABI to allow testing special edge case properties like rounding. For more info, see the ERC4626 readme.

ABDKMath64x64 tests

The Solidity smart contract programming language does not have any inbuilt feature for working with decimal numbers, so for contracts dealing with non-integer values, a third party solution is needed. ABDKMath64x64 is a fixed-point arithmetic Solidity library that operates on 64.64-bit numbers.

A 64.64-bit fixed-point number is a data type that consists of a sign bit, a 63-bit integer part, and a 64bit decimal part. Since there is no direct support for fractional numbers in the EVM, the underlying data type that stores the values is a 128-bit signed integer.

ABDKMath64x64 library implements 19 arithmetic operations using fixed-point numbers and 6 conversion functions between integer types and fixed-point types.

We provide a number of tests related with fundamental mathematical properties of the floating point numbers. To include these tests into your repository, follow these steps:

  1. Integration
  2. Run

Integration

Create a new Solidity file containing the ABDKMath64x64Harness contract:

pragma solidity ^0.8.0;
import "@crytic/properties/contracts/Math/ABDKMath64x64/ABDKMath64x64PropertyTests.sol;

contract CryticABDKMath64x64Harness is CryticABDKMath64x64PropertyTests {
    /* Any additional test can be added here */
}

Run

Run the test suite using echidna . --contract CryticABDKMath64x64Harness --seq-len 1 --test-mode assertion --corpus-dir tests/echidna-corpus and inspect the coverage report in tests/echidna-corpus when it finishes.

Additional resources

Helper functions

The repository provides a collection of functions and events meant to simplify the debugging and testing of assertions in Echidna. Commonly used functions, such as integer clamping or logging for different types are available in contracts/util/PropertiesHelper.sol.

Available helpers:

  • LogXxx: Events that can be used to log values in fuzzing tests. string, uint256 and address loggers are provided. In Echidna's assertion mode, when an assertion violation is detected, all events emitted in the call sequence are printed.
  • assertXxx: Asserts that a condition is met, logging violations. Assertions for equality, non-equality, greater-than, greater-than-or-equal, less-than and less-than-or-equal comparisons are provided, and user-provided messages are supported for logging.
  • clampXxx: Limits an int256 or uint256 to a certain range. Clamps for less-than, less-than-or-equal, greater-than, greater-than-or-equal, and range are provided.

Usage examples

Logging

Log a value for debugging. When the assertion is violated, the value of someValue will be printed:

pragma solidity ^0.8.0;

import "@crytic/properties/contracts/util/PropertiesHelper.sol";

contract TestProperties is PropertiesAsserts {
  // ...

  function test_some_invariant(uint256 someValue) public {
    // ...

    LogUint256("someValue is: ", someValue);

    // ...

    assert(fail);

    // ...
  }

  // ...
}

Assertions

Assert equality, and log violations:

pragma solidity ^0.8.0;

import "@crytic/properties/contracts/util/PropertiesHelper.sol";

contract TestProperties is PropertiesAsserts {
  // ...

  function test_some_invariant(uint256 someValue) public {
    // ...

    assertEq(someValue, 25, "someValue doesn't have the correct value");

    // ...
  }

  // ...
}

In case this assertion fails, for example if someValue is 30, the following will be printed in Echidna:

Invalid: 30!=25, reason: someValue doesn't have the correct value

Clamping

Assure that a function's fuzzed parameter is in a certain range:

pragma solidity ^0.8.0;

import "@crytic/properties/contracts/util/PropertiesHelper.sol";

contract TestProperties is PropertiesAsserts {
  int256 constant MAX_VALUE = 2 ** 160;
  int256 constant MIN_VALUE = -2 ** 24;

  // ...

  function test_some_invariant(int256 someValue) public {
    someValue = clampBetween(someValue, MIN_VALUE, MAX_VALUE);

    // ...
  }

  // ...
}

HEVM cheat codes support

Since version 2.0.5, Echidna supports HEVM cheat codes. This repository contains a Hevm.sol contract that exposes cheat codes for easy integration into contracts under test.

Cheat codes should be used with care, since they can alter the execution environment in ways that are not expected, and may introduce false positives or false negatives.

Usage example

Use prank to simulate a call from a different msg.sender:

pragma solidity ^0.8.0;

import "@crytic/properties/contracts/util/Hevm.sol";

contract TestProperties {
  // ...

  function test_some_invariant(uint256 someValue) public {
    // ...

    hevm.prank(newSender);
    otherContract.someFunction(someValue); // This call's msg.sender will be newSender
    otherContract.someFunction(someValue); // This call's msg.sender will be address(this)

    // ...
  }

  // ...
}

Trophies

A list of security vulnerabilities that were found using the properties can be found on the trophies page.

How to contribute to this repo?

Contributions are welcome! You can read more about the contribution guidelines and directory structure in the CONTRIBUTING.md file.

properties's People

Contributors

0xmichalis avatar audityourcontracts avatar aviggiano avatar bsamuels453 avatar chmielewskikamil avatar ggrieco-tob avatar gianfrancobazzani avatar glarregay-tob avatar montyly avatar oldsj avatar smonicas avatar tuturu-tech avatar yanhuijessica avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

properties's Issues

[Feature-request]: Merkle trie properties

Describe the desired feature or improvement

  1. How challenging would it be to create a generic set of merkle trie properties?
  2. Do merkle trie library interfaces have enough commonalities for this kind of test suite to be possible?

Standardize the properties

We need to standardize the properties in this repo, for example:

We should also agree on a structure to group the properties in the file, and how to document the sections (ex

/* ================================================================
TESTS FOR FUNCTION sub()
================================================================ */
)

Once we have defined the rules, we should apply them on the repo, and integrate them in the contribution guidelines

Create helpers for non standard and edge case of erc20

Which would cover everything in https://github.com/crytic/building-secure-contracts/blob/master/development-guidelines/token_integration.md#erc20-tokens, plus other edge cases we are aware of

The idea would be to have helpers to ease the integration of erc20 edge case, like

import "@crytic/properties/contracts/util/erc20/erc20.sol";

Which would include :

  • All the different ERC20
  • all_erc20_standard() returns ( IERC20[] memory) - returns all standards token deployed, and converted to IERC20 object (or a similar name)
  • all_erc20_non_standard() returns ( IERC20[] memory) - returns all non-standard token deployed, and converted to IERC20 object (or a similar name)
  • all_erc20() returns ( IERC20[] memory) - returns all tokens deployed, and converted to IERC20 object (or a similar name)

[Feature-request]: Refactor contract names to match contract file names

Describe the desired feature or improvement

This is a bit subjective, but I find it confusing:

import {PropertiesAsserts} from "@crytic/properties/contracts/util/PropertiesHelper.sol";

I propose a refactor so that the contract name matches the contract filename:

import {PropertiesAsserts} from "@crytic/properties/contracts/util/PropertiesAsserts.sol";

[Feature-request]: foundry-compatible assertion helpers

Describe the desired feature or improvement

When integrating https://github.com/crytic/properties with an existing foundry codebase, many assertion helpers collide with forge-std/Test.sol, which makes it harder to use both helpers on the same set of contracts.

For example, this repo's assertWithMsg from this repo serves the same purpose as foundry's assertTrue, so they could have the same name. Also, one uses assertGte while the other uses assertGe, etc.

It would be nice if this repo was fully compatible with foundry for an easier integration.

`usedId` purpose in `ERC721`

In the IERC721Internal interface you define the following function usedId:

function usedId(uint256 tokenId) external view returns (bool);

I'm having a hard time to understand what is the reason behind using this as part of the interface. In your ERC721Compliant contract, you declare it but not use it:

mapping(uint256 => bool) public usedId;

If someone adds something like usedId[id] = true to the _customMint function, the following property always fails obviously:

assertWithMsg(!token.usedId(tokenId), "Token ID minted is not new");

If you want to track the used ids, you should update it after the above test assertion to make sense.

Furthermore, the README section on the ERC721 is outdated. There is no ITokenMock in the ERC721 case for example, but IERC721Internal. I would recommend making it consistent with the ERC20 example tho & updating the docs about the latest status quo.

[Bug-Candidate]: Rename "clamp" to avoid confusion

Describe the issue:

The state

There's a set of functions in contracts/util/PropertiesHelper.sol named clamp... for making numbers fit in a fixed range by performing modulo. For example using such function to put numbers 0-10 in the range 4-6 will result in:

in:  0 1 2 3 4 5 6 7 8 9 10
out: 6 4 5 6 4 5 6 4 5 6 4

The problem

In the programming nomenclature the term "clamp" usually refers to putting number in a fixed range by assigning it the value of the boundary, if that boundary is exceeded. For example clamping numbers 0-10 in the range 4-6 would result in:

in:  0 1 2 3 4 5 6 7 8 9 10
out: 4 4 4 4 4 5 6 6 6 6 6

I'm writing this based on my experience, I got really confused when I saw PropertyHelper's clamp... functions in use for the first time. I checked my expectations by searching on the internet for "clamp a number", and all the resources were referring to "clamping" as assigning the exceeded boundary, not calculating the modulo. I think that many developers using PropertiesHelper for the first time will be confused or misled.

The solution

I think that the clamp... functions should be renamed to something less confusing. For example the name wrap... could be good, it would reflect how numbers exceeding the limit are returning into the range on the other end.

Steps to reproduce the issue:

Write code using PropertiesHelper.

If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

No response

Echidna version:

All of them.

Additional information:

No response

[Feature-request]: NFT minting properties

Describe the desired feature or improvement

I am reviewing a smart contract that contains a very complex logic to mint NFTs with random rarities using Chainlink VRF. Just by reading the code it is hard to understand if it correctly implements the specification. It would be nice to have pre-built echidna properties to test invariants.

In my particular example, the invariants are rather commonplace to other NFT projects, which is why I believe these are generic enough to be applied to other smart contracts:

  • max NFTs per wallet: no user can have more NFTs than the max
  • max NFTs minted: total supply must not exceed the max
  • max NFTs per project owner: the project owner cannot have more than the max
  • NFT price (in Ether, but can be extended to ERC-20 too): minting an NFT must cost exactly the NFT price
  • NFT rarity: the number of NFTs minted for a given rarity must match their assigned percentages, within an error margin
  • whitelist: no user can mint NFTs if they are not on the whitelist
  • etc

If you think this is useful I can work on these properties and submit a PR.

[Bug-Candidate]: `PropertiesConstants.INITIAL_BALANCE` is an arbitrary value unrelated to Echidna default setup

Describe the issue:

The PropertiesConstants contracts contain some hardcoded parameters that are related to Echidna's default setup, such as USER1, USER2, and USER3.

The issue is that it also contains INITIAL_BALANCE, which has nothing to do with balanceAddr or balanceContract. This can be confusing to users.

Maybe it is better to migrate this value to another contract and create the related BALANCE_ADDR or BALANCE_CONTRACT variables.

Steps to reproduce the issue:

N/A

If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

N/A

Echidna version:

N/A

Additional information:

N/A

[Bug-Candidate]: ABDKMath64x64PropertyTests.sol: Tests output false negatives

Describe the issue:

These 2 tests:

contract CryticABDKMath64x64Properties {
    ...
    function add_test_range(int128 x, int128 y) public view {
        int128 result;
        try this.add(x, y) {
            result = this.add(x, y);
            assert(result <= MAX_64x64 && result >= MIN_64x64);
        } catch {
            // If it reverts, just ignore
        }
    }

    function sub_test_range(int128 x, int128 y) public view {
        int128 result;
        try this.sub(x, y) {
            result = this.sub(x, y);
            assert(result <= MAX_64x64 && result >= MIN_64x64);
        } catch {
            // If it reverts, just ignore
        }
    }
}

They are not actually showing failure in case this.add/this.sub overflow or underflow.
It looks like result being int128 makes it always conforming with the assert statement despite the failure in this.add/this.sub.

Steps to reproduce the issue:

  • Create a smart contract to contain the test in ``tests/ABDKMath64x64PropertyTests/hardhat/contracts/CryticMathTest.sol`
pragma solidity ^0.8.0;
import "@crytic/properties/contracts/Math/ABDKMath64x64/ABDKMath64x64PropertyTests.sol";

contract CryticABDKMath64x64Harness is CryticABDKMath64x64Properties {
    /* Any additional test can be added here */
}
  • Working directory: tests/ABDKMath64x64PropertyTests/hardhat

  • Force ABDKMath64x64 to overflow/underflow by commenting out the require statement on the tested methods.

library ABDKMath64x64 {
   ....
  function add (int128 x, int128 y) internal pure returns (int128) {
    unchecked {
      int256 result = int256(x) + y;
      // COMMENTED THIS TO FORCE IT TO FAIL
      //  require (result >= MIN_64x64 && result <= MAX_64x64);
      return int128 (result);
    }
  }
  function sub (int128 x, int128 y) internal pure returns (int128) {
    unchecked {
      int256 result = int256(x) - y;
      // COMMENTED THIS TO FORCE IT TO FAIL
      //  require (result >= MIN_64x64 && result <= MAX_64x64);
      return int128 (result);
    }
  }
}
  • Run echidna-test . --contract CryticABDKMath64x64Harness --test-mode assertion

  • Output:

sub_test_range(int128,int128):  passed! ๐ŸŽ‰
abs(int128):  passed! ๐ŸŽ‰
add_test_maximum_value():  passed! ๐ŸŽ‰
mulu(int128,uint256):  passed! ๐ŸŽ‰
add_test_range(int128,int128):  passed! ๐ŸŽ‰
  • Some other tests failed due to that change but add_test_range and sub_test_range did not fail as expected.

If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

No response

Echidna version:

Echidna 2.0.1

Additional information:

No response

[Bug-Candidate]: Difference in ERC721BurnableProperties for external vs. internal

Describe the issue:

CryticERC721ExternalBurnableProperties contains 5 tests while CryticERC721BurnableProperties contains 6 tests. This means that the list of 19 property tests for ERC721 is actually only 18 property tests for the external tests. Either a test is forgotten in the external tests or the list of property tests should differ for external vs internal.

Steps to reproduce the issue:

The external tests only have one test test_ERC721_external_burnRevertOnTransfer while the internal tests have test_ERC721_burnRevertOnTransferFromPreviousOwner and test_ERC721_burnRevertOnTransferFromZeroAddress.

If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

No response

Echidna version:

2.2.3

Additional information:

No response

[Feature-request]: MultiSig wallet properties

Describe the desired feature or improvement

Since a lot of MultiSigs work in a relatively similar way (e.g., using signatures, some threshold of votes/signatures is required, only approved signers can execute, etc.) we could probably build a set of general properties to test MultiSigs.

For example:

  • If votes < threshold, calls can't be executed
  • Signatures cannot be reused
  • nonce is strictly monotonously increasing
  • nonce can only be used once

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.