filecoin-project / filecoin-solidity

Filecoin Solidity API Library
Other
17 stars 11 forks source link

[M4] Optimizations and #55 fix #56

Closed wertikalk closed 2 months ago

wertikalk commented 4 months ago

closes #45 closes #55

snissn commented 3 months ago

There's some changes to this PR from m4 that no longer lets forge run with the normal compilation argument and requires --via-ir which is implying that there is a regression somewhere:

% forge t
[⠊] Compiling...
[⠊] Compiling 58 files with Solc 0.8.18
[⠒] Solc 0.8.18 finished in 2.01s
Error: 
Compiler run failed:
Error: Compiler error (/solidity/libyul/backends/evm/AsmCodeGen.cpp:68):Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables. When compiling inline assembly: Variable dataEnd is 1 slot(s) too deep inside the stack. Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.
CompilerError: Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables. When compiling inline assembly: Variable dataEnd is 1 slot(s) too deep inside the stack. Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.
snissn commented 3 months ago

I've looked a bit into adding the hardhat gas reporter plugin to hardhat to run the tests with before and after this PR but don't have that working at the moment

wertikalk commented 3 months ago

There's some changes to this PR from m4 that no longer lets forge run with the normal compilation argument and requires --via-ir which is implying that there is a regression somewhere:

% forge t
[⠊] Compiling...
[⠊] Compiling 58 files with Solc 0.8.18
[⠒] Solc 0.8.18 finished in 2.01s
Error: 
Compiler run failed:
Error: Compiler error (/solidity/libyul/backends/evm/AsmCodeGen.cpp:68):Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables. When compiling inline assembly: Variable dataEnd is 1 slot(s) too deep inside the stack. Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.
CompilerError: Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables. When compiling inline assembly: Variable dataEnd is 1 slot(s) too deep inside the stack. Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.

The problem was in reorder of a nested struct inside market types. Pushed the fix.

snissn commented 3 months ago

I see failures when I merge m4 into this branch and then run make for the calibnet tests:


# make test_hh_calibnet
export HH_NETWORK=calibnet && npx hardhat test

  Account Test
    1) Test 1: Integration test port
DBG: (Test 1: Integration test port) :::: 
    Deploying contracts... (account)

    Authenticating message...

  Address Test
    ✔ Test 1: Integration test port (129278ms)

  BigInt Test
    ✔ Test 1: Integration test port (108927ms)

  CborDecode Test
    ✔ Test 1: Integration test port (161682ms)

  Datacap Test
    ✔ Test 1: Integration test port (79257ms)

  Deserialize Params Test
    ✔ Test 1: Integration test port (130248ms)

  Leb128 Test
    ✔ Test 1: Integration test port

  Market Test
    2) Test 1: State changes on Calibnet
DBG: (Test 1: State changes on Calibnet) :::: 
    Adding funds to balance...

    ✔ Test 2: Reading from Calibnet (5432ms)

  Market Cbot Test
    ✔ Test 1: Integration test port (142391ms)

  Precompiles Test
    ✔ Test 1: Integration test port (111770ms)

  Send Test
    ✔ Test 1: Integration test port (215227ms)

  Verifreg Test
    ✔ Test 1: Integration test port (197015ms)

  Market Tests (Beacon)
    3) "before all" hook for "Test 1: State changes on Calibnet (Before Upgrade)"

  Market Tests (Transparent)
    4) Test 1: State changes on Calibnet (Before Upgrade)
DBG::No logs for: 'Test 1: State changes on Calibnet (Before Upgrade)'
    5) Test 2: Reading from Calibnet (Before Upgrade)
DBG::No logs for: 'Test 2: Reading from Calibnet (Before Upgrade)'
    6) Test 1: State changes on Calibnet (After Upgrade)
DBG::No logs for: 'Test 1: State changes on Calibnet (After Upgrade)'
    7) Test 2: Reading from Calibnet (After Upgrade)
DBG::No logs for: 'Test 2: Reading from Calibnet (After Upgrade)'

  Market Tests (UUPS)
    8) Test 1: State changes on Calibnet (Before Upgrade)
DBG::No logs for: 'Test 1: State changes on Calibnet (Before Upgrade)'
    9) Test 2: Reading from Calibnet (Before Upgrade)
DBG::No logs for: 'Test 2: Reading from Calibnet (Before Upgrade)'
    10) Test 1: State changes on Calibnet (After Upgrade)
DBG::No logs for: 'Test 1: State changes on Calibnet (After Upgrade)'
    11) Test 2: Reading from Calibnet (After Upgrade)
DBG::No logs for: 'Test 2: Reading from Calibnet (After Upgrade)'

  11 passing (32m)
  11 failing

  1) Account Test
       Test 1: Integration test port:
     Error: missing revert data (action="call", data=null, reason=null, transaction={ "data": "0xf5c62e87000000000000000000000000000000000000000000000000000000000001b3370000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000060b179ef5bdedaffffdd5a8c245c7e3a9629329b7870ee56eff12dbbc5479cebaae70233d708b37d1ff57ccab813c22aa80a9ea5d8fd8df5364d3dc71e024e1ffd872d87c2c60cab54966aac4f6b1fee4b0b3c663dde7d6d525a26f57e569452c500000000000000000000000000000000000000000000000000000000000000608eabea2a4001061ac4c9fe3c517725b8829b159149a863b2a2320cc628d026a871d3cb34947371f384a9eb49ff9bd56a019fa70e10c06ac5ca93df3c1d6f54d540c57cbe2f5209cafdc12146d5d59172dd4d8359015e10584fa6327de0ce5a6a", "from": "0xf7931ff7FC55d19EF4A8139fa7E4b3F06e03F2e2", "to": "0x0BFDECACaffD4edf8a7c901D766B5EAF20A19211" }, invocation=null, revert=null, code=CALL_EXCEPTION, version=6.12.1)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at getBuiltinCallException (node_modules/ethers/src.ts/abi/abi-coder.ts:118:21)
      at Function.getBuiltinCallException (node_modules/ethers/src.ts/abi/abi-coder.ts:235:16)
      at JsonRpcProvider.getRpcError (node_modules/ethers/src.ts/providers/provider-jsonrpc.ts:979:32)
      at /var/lib/fil-sol/node_modules/ethers/src.ts/providers/provider-jsonrpc.ts:563:45
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

  2) Market Test
       Test 1: State changes on Calibnet:

      AssertionError: expected '0x00' to equal '0x0de0b6b3a7640000'
      + expected - actual

      -0x00
      +0x0de0b6b3a7640000

      at /var/lib/fil-sol/hh-test/calibnet/e2e/market.t.ts:62:48
      at step (hh-test/calibnet/e2e/market.t.ts:33:23)
      at Object.next (hh-test/calibnet/e2e/market.t.ts:14:53)
      at fulfilled (hh-test/calibnet/e2e/market.t.ts:5:58)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

  3) Market Tests (Beacon)
       "before all" hook for "Test 1: State changes on Calibnet (Before Upgrade)":
     Error: Contract at 0xe9777E3eC5f3C741EA13670e4af7A1c651560CAb doesn't look like a beacon

Deploy a beacon using deployBeacon().
      at Proxy.deployBeaconProxy (node_modules/@openzeppelin/hardhat-upgrades/src/deploy-beacon-proxy.ts:71:13)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

  4) Market Tests (Transparent)
       Test 1: State changes on Calibnet (Before Upgrade):
     Error: could not decode result data (value="0x", info={ "method": "get_balance", "signature": "get_balance((bytes))" }, code=BAD_DATA, version=6.12.1)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25)
      at Interface.decodeFunctionResult (node_modules/ethers/src.ts/abi/interface.ts:916:15)
      at staticCallResult (node_modules/ethers/src.ts/contract/contract.ts:346:35)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async staticCall (node_modules/ethers/src.ts/contract/contract.ts:303:24)
      at async Proxy.get_balance (node_modules/ethers/src.ts/contract/contract.ts:351:41)

  5) Market Tests (Transparent)
       Test 2: Reading from Calibnet (Before Upgrade):
     Error: could not decode result data (value="0x", info={ "method": "get_deal_data_commitment", "signature": "get_deal_data_commitment(uint64)" }, code=BAD_DATA, version=6.12.1)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25)
      at Interface.decodeFunctionResult (node_modules/ethers/src.ts/abi/interface.ts:916:15)
      at staticCallResult (node_modules/ethers/src.ts/contract/contract.ts:346:35)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async staticCall (node_modules/ethers/src.ts/contract/contract.ts:303:24)
      at async Proxy.get_deal_data_commitment (node_modules/ethers/src.ts/contract/contract.ts:351:41)

  6) Market Tests (Transparent)
       Test 1: State changes on Calibnet (After Upgrade):
     Error: Contract at 0x0766F517D18b362C90D78a3B7Ef0982C3233Aa2F doesn't look like an ERC 1967 proxy with a logic contract address

      at getImplementationAddress (node_modules/@openzeppelin/upgrades-core/src/eip-1967.ts:29:11)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async processProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:23:26)
      at async validateProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:62:30)
      at async deployProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:72:3)
      at async Proxy.upgradeProxy (node_modules/@openzeppelin/hardhat-upgrades/src/upgrade-proxy.ts:38:32)

  7) Market Tests (Transparent)
       Test 2: Reading from Calibnet (After Upgrade):
     Error: Contract at 0x0766F517D18b362C90D78a3B7Ef0982C3233Aa2F doesn't look like an ERC 1967 proxy with a logic contract address

      at getImplementationAddress (node_modules/@openzeppelin/upgrades-core/src/eip-1967.ts:29:11)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async processProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:23:26)
      at async validateProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:62:30)
      at async deployProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:72:3)
      at async Proxy.upgradeProxy (node_modules/@openzeppelin/hardhat-upgrades/src/upgrade-proxy.ts:38:32)

  8) Market Tests (UUPS)
       Test 1: State changes on Calibnet (Before Upgrade):
     Error: could not decode result data (value="0x", info={ "method": "get_balance", "signature": "get_balance((bytes))" }, code=BAD_DATA, version=6.12.1)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25)
      at Interface.decodeFunctionResult (node_modules/ethers/src.ts/abi/interface.ts:916:15)
      at staticCallResult (node_modules/ethers/src.ts/contract/contract.ts:346:35)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async staticCall (node_modules/ethers/src.ts/contract/contract.ts:303:24)
      at async Proxy.get_balance (node_modules/ethers/src.ts/contract/contract.ts:351:41)

  9) Market Tests (UUPS)
       Test 2: Reading from Calibnet (Before Upgrade):
     Error: could not decode result data (value="0x", info={ "method": "get_deal_data_commitment", "signature": "get_deal_data_commitment(uint64)" }, code=BAD_DATA, version=6.12.1)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25)
      at Interface.decodeFunctionResult (node_modules/ethers/src.ts/abi/interface.ts:916:15)
      at staticCallResult (node_modules/ethers/src.ts/contract/contract.ts:346:35)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async staticCall (node_modules/ethers/src.ts/contract/contract.ts:303:24)
      at async Proxy.get_deal_data_commitment (node_modules/ethers/src.ts/contract/contract.ts:351:41)

  10) Market Tests (UUPS)
       Test 1: State changes on Calibnet (After Upgrade):
     Error: Contract at 0x344DC84a6D161Fc5aDB94257a80AD352607749b6 doesn't look like an ERC 1967 proxy with a logic contract address

      at getImplementationAddress (node_modules/@openzeppelin/upgrades-core/src/eip-1967.ts:29:11)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async processProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:23:26)
      at async validateProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:62:30)
      at async deployProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:72:3)
      at async Proxy.upgradeProxy (node_modules/@openzeppelin/hardhat-upgrades/src/upgrade-proxy.ts:38:32)

  11) Market Tests (UUPS)
       Test 2: Reading from Calibnet (After Upgrade):
     Error: Contract at 0x344DC84a6D161Fc5aDB94257a80AD352607749b6 doesn't look like an ERC 1967 proxy with a logic contract address

      at getImplementationAddress (node_modules/@openzeppelin/upgrades-core/src/eip-1967.ts:29:11)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async processProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:23:26)
      at async validateProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:62:30)
      at async deployProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:72:3)
      at async Proxy.upgradeProxy (node_modules/@openzeppelin/hardhat-upgrades/src/upgrade-proxy.ts:38:32)

make: *** [Makefile:154: test_hh_calibnet] Error 11
snissn commented 3 months ago

I think the rust tests are failing in this branch:

make test_integration

..

     Running tests/account.rs (target/debug/deps/account-c6de2ac08d647dcb)

running 1 test
+-------------------------+---------+
| Function                | Gas     |
+-------------------------+---------+
| authenticate_message    | 5914808 |
+-------------------------+---------+
| universal_receiver_hook | 4317088 |
+-------------------------+---------+
test account_tests ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.20s

     Running tests/address.rs (target/debug/deps/address-0fc29c49a9ca3d29)

running 1 test
test address_tests ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.36s

     Running tests/bigints.rs (target/debug/deps/bigints-eb6f31ef8e3b64cd)

running 1 test
test bigints_tests ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.56s

     Running tests/cborDecode.rs (target/debug/deps/cborDecode-059724a7f613e430)

running 1 test
^Tretest cbor_decode_tests ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.62s

     Running tests/datacap.rs (target/debug/deps/datacap-28a8f39c2464dd2e)

running 1 test
test datacap_tests ... FAILED

failures:

---- datacap_tests stdout ----
Testing solidity API
Embryo address delegated type [f410f3l7kjewzyzztvy6vnn7ndlnwa2jmtc6fdrqoc5i]
Embryo address delegated type on hex [040adafea492d9c6733ae3d56b7ed1adb60692c98bc5]
Sender address id [100] and bytes [011eda43d05ca6d7d637e7065ef6b8c5db89e5fb0c]
Sender address id [101] and bytes [01dce5b7f69e73494891556a350f8cc357614916d5]
Sender address id [102] and bytes [017cd9e22fa2f01d24d2e9e91d8519aff2e5d65a62]
Sender address id [103] and bytes [01a53e34d73bbd7688a8d8be9448b2ede303349e30]
Try to call constructor on verifreg actor
Try to call constructor on datacap actor
Calling init actor (EVM)
[TRACE]<Init::1> delegated address: Address { payload: Delegated(DelegatedAddress { namespace: 10, length: 20, buffer: [250, 95, 11, 20, 240, 126, 8, 61, 236, 9, 71, 215, 13, 47, 252, 83, 67, 47, 105, 54, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }) }
[TRACE]<Init::1> robust address: Address { payload: Actor([108, 33, 81, 160, 92, 85, 234, 134, 70, 129, 189, 252, 59, 100, 129, 97, 15, 204, 16, 189]) }
Contract address ID type on decimal [401]
Contract address ID type on hex [009103]
Contract address robust type [f2nqqvdic4kxvimrubxx6dwzebmeh4yef5fz6cwla]
Contract address eth address type [fa5f0b14f07e083dec0947d70d2ffc53432f6936]
Minting some tokens on datacap actor
Minting more tokens on datacap actor
Calling `name`
[INFO]<EVMContract::401> Call Precompile:
    address: fe00000000000000000000000000000000000005
    context: PrecompileContext { call_type: DelegateCall, gas_limit: 980994278, value: 0 }
    input: 0000000000000000000000000000000000000000000000000000000002ea015c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000070000000000000000000000000000000000000000000000000000000000000000
Calling `symbol`
[INFO]<EVMContract::401> Call Precompile:
    address: fe00000000000000000000000000000000000005
    context: PrecompileContext { call_type: DelegateCall, gas_limit: 980991321, value: 0 }
    input: 000000000000000000000000000000000000000000000000000000007adab63e00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000070000000000000000000000000000000000000000000000000000000000000000
Calling `total_supply`
[INFO]<EVMContract::401> Call Precompile:
    address: fe00000000000000000000000000000000000005
    context: PrecompileContext { call_type: DelegateCall, gas_limit: 980965475, value: 0 }
    input: 0000000000000000000000000000000000000000000000000000000006da7a3500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000070000000000000000000000000000000000000000000000000000000000000000
Calling `balance`
[INFO]<EVMContract::401> Call Precompile:
    address: fe00000000000000000000000000000000000005
    context: PrecompileContext { call_type: DelegateCall, gas_limit: 980389982, value: 0 }
    input: 00000000000000000000000000000000000000000000000000000000c26ddbd500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000005100000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000000700000000000000000000000000000000000000000000000000000000000000034200420000000000000000000000000000000000000000000000000000000000
Calling `allowance`
[INFO]<EVMContract::401> Call Precompile:
    address: fe00000000000000000000000000000000000005
    context: PrecompileContext { call_type: DelegateCall, gas_limit: 979802626, value: 0 }
    input: 00000000000000000000000000000000000000000000000000000000faa4523600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000005100000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000007000000000000000000000000000000000000000000000000000000000000002d8255011eda43d05ca6d7d637e7065ef6b8c5db89e5fb0c5501dce5b7f69e73494891556a350f8cc357614916d500000000000000000000000000000000000000
Calling `transfer`
thread 'datacap_tests' panicked at tests/datacap.rs:499:5:
assertion `left == right` failed
  left: 33
 right: 0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    datacap_tests

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 17.32s

error: test failed, to rerun pass `--test datacap`
wertikalk commented 2 months ago

closing in favor of: https://github.com/filecoin-project/filecoin-solidity/pull/63