Martinvol/isolate 0.8 artifacts for gasPriceMinimum #10437

martinvol · 2023-07-26T10:41:06Z

Description

This PR fixes key features of our tooling to enable the tooling to be able to deploy 0.8 Solidity. The keys to the approach are:

Truffle now has two folders for contracts, one for all 0.5.* contracts and other for 0.8.*. Contracts that are shared were were left in the original folder, but their pragma was made more flexible (but truffle will compile with the specified compiler).
a. The reason to have separate folders is that if a contract is built, it compiles all its dependencies with the solidity version of the contract, but if those were already built for a different smart contract, the artifacts get overwritten, making the storage check fail.
Truffle now has two configuration files, one for 0.5.* and one for 0.8.*, using automatic "pragma" setting will cause the error described in 1.a
Artifacts once built also land in a separate folder to avoid collisions, this is a similar approach to the one used when removing mento contracts.
A big chunk of the PR is making the tooling aware that now artifacts are in more than one folder, those a bunch of function inputs were changed to arrays
Libraries were removed because they had files (name SafeMath), that were overwriting or own, and thus causing problems.
Another important consideration is that GasPriceMinimum has a new solidity version and it's in a different folder, but its proxy it's still the same as before. This is because code changes will be needed to make it compatible with 0.8. For that reason, many helpers were created to specify the proxy artifact.

TODO:

Make it build with multiple Solidity versions going to different artifacts

Fix tests (at this point there's a lot of flakiness, but only a couple actually fail)
Make sure verify-deployed, verify-release works, make-release and check-versions work. (Volpe: I tested it but right before merging it should be done again)
have two truffle configs sharing a single parent
Clean up code, remove TODO's
Clean up code, remove console.log

Other changes

Describe any minor or "drive-by" changes here.

Tested

An explanation of how the changes were tested or an explanation as to why they don't need to be.

Related issues

Fixes #[issue number here]

Backwards compatibility

Brief explanation of why these changes are/are not backwards compatible.

Documentation

The set of community facing docs that have been added/modified because of this change

… (bc they were not updated)

socket-security · 2023-07-26T10:45:18Z

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: @summa-tx/[email protected]

martinvol · 2023-07-26T11:17:48Z

packages/protocol/scripts/truffle/make-release.ts

+      try {
+        contractArtifact = await artifacts.require(contractName) // I think it won't be able to find GasPriceMinimum here
+      } catch {
+        // it wasn't found in the standar artifacts folder, check if it's


This needs to be generalized

packages/protocol/scripts/truffle/make-release.ts

soloseng

Looks good! Made a few comment.

To keep the protocol folder structure simple, would it be possible to organize contracts with a subdirectory for v0.5 & v0.8.

packages/protocol/truffle-config0.8.js

soloseng · 2023-07-26T19:50:41Z

packages/protocol/contracts/common/libraries/SafeMathMem.sol

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: MIT
+// Original authors: https://github.com/summa-tx/memview-sol
+pragma solidity >=0.5.10;


should this version be updated to pragma solidity >=0.5.10 <0.8.0;?

packages/protocol/test/compatibility/ast-code.ts

m-chrzan · 2023-07-31T15:16:44Z

packages/protocol/contracts-0.8/UsingRegistry8.sol

@@ -68,43 +57,7 @@ contract UsingRegistry is Ownable {
    emit RegistrySet(registryAddress);
  }

-  function getAccounts() internal view returns (IAccounts) {


Just wondering, why does this remove a bunch of the getters?

Also, is there a reason this is modeled after the original UsingRegistry rather than the more efficient UsingRegistryV2?

It's done like this so I didn't have to make the interfaces have permissive pragmas.

Regarding the UsingRegistryV2, I'm not sure, the first implementation used this one, but I'll take a look.

but now thinking about this, we already have flexible pragmas and everything lands in the right folder, so I guess I could bring it back. Will try that, thanks for the comment.

Just remembered why not all getters could are there, because Mento's ones can be used with 0.8.

It is not using UsingRegistryV2 because gasPriceMinimum is using UsingRegistry.

packages/protocol/contracts-0.8/UsingRegistry8.sol

packages/protocol/contracts/common/libraries/SafeMathMem.sol

packages/protocol/lib/bytecode.ts

m-chrzan · 2023-07-31T15:36:05Z

packages/protocol/lib/bytecode.ts

+    console.log("this.addresses[library]", this.addresses[library])
+    console.log("address", address)


Debug logs to be removed?

After I'm done :)
(verify release still doesn't work)

packages/protocol/lib/compatibility/report.ts

m-chrzan · 2023-07-31T15:43:50Z

packages/protocol/lib/compatibility/report.ts

        reportIndex.libraries[contract] = changesByContract[contract]
      } else {
        const report = ASTVersionedReport.create(changesByContract[contract])
        reportIndex.contracts[contract] = report
      }
    })
+    console.log("made it here b")


Ditto here and elsewhere, but I guess it's still a WIP PR.

packages/protocol/scripts/build.ts

packages/protocol/lib/compatibility/ast-code.ts

…-0.8-artifacts-2

…/celo-monorepo into martinvol/isolate-0.8-artifacts

martinvol · 2023-09-12T18:48:22Z

packages/cli/src/commands/releasegold/withdraw.test.ts

@@ -28,7 +28,7 @@ testWithGanache('releasegold:withdraw cmd', (web3: Web3) => {
  test('can withdraw released gold to beneficiary', async () => {
    await testLocally(SetLiquidityProvision, ['--contract', contractAddress, '--yesreally'])
    // ReleasePeriod of default contract
-    await timeTravel(100000000, web3)
+    await timeTravel(300000000, web3)


This fix is cherry-picked from master, right @pahor167

packages/protocol/migrationsConfig.js

…acts

martinvol added 12 commits July 21, 2023 17:32

Added different compilation folder for 0.8 contracts

65bea52

Add check to see if folder is there

d57a615

Check that there are 0.8 contracts to build before trying

10220b8

nits

26af04c

Removed duplicated contracts

81ac022

Attempt to fix conflict by removing library TypedMem

c12fc99

Fixed version

7ec94ea

updated versions

c4d28fe

Made the report look for many artifacts folders, test will still fail…

6de5d05

… (bc they were not updated)

fixed tests

cdd44a7

dry run now works

a14e4bd

The tooling now works

7f0978f

martinvol added 5 commits July 26, 2023 12:54

lint

eac63bb

fix build error

7a5574f

builds

1756e00

Added licence to files

7fc51ca

comment types

7a475b1

martinvol commented Jul 26, 2023

View reviewed changes

packages/protocol/scripts/truffle/make-release.ts Show resolved Hide resolved

soloseng reviewed Jul 26, 2023

View reviewed changes

martinvol added 4 commits July 27, 2023 15:23

Added broken build script

028112b

modified contract packages

e3e305a

wip

6204e01

Added truffle file

82a80db

m-chrzan reviewed Jul 31, 2023

View reviewed changes

martinvol added 4 commits August 11, 2023 15:00

Added back sorted Oracles

05c2a68

Bytecode removed sortedOracles

30f8483

All the commands work

1bdeb5f

Merge branch 'martinvol/isolate-0.8-artifacts' into martinvol/isolate…

e9899f7

…-0.8-artifacts-2

martinvol and others added 3 commits September 6, 2023 17:45

Merge branch 'martinvol/isolate-0.8-artifacts' of github.com:celo-org…

59fc3e6

…/celo-monorepo into martinvol/isolate-0.8-artifacts

stability + verbose logging

158d38e

Merge branch 'martinvol/isolate-0.8-artifacts' of github.com:celo-org…

3e9df8e

…/celo-monorepo into martinvol/isolate-0.8-artifacts

pahor167 requested a review from a team as a code owner September 7, 2023 13:41

pahor167 requested a review from ssubram11 September 7, 2023 13:41

lint fix

1b27fdc

martinvol commented Sep 12, 2023

View reviewed changes

martinvol added 3 commits September 14, 2023 12:25

Skiped CIP35 tets

6c3474b

Added issue contractPackages.ts

a4a9e63

Clarified TODO Uniswap mock

4015dbc

martinvol commented Sep 15, 2023

View reviewed changes

packages/protocol/migrationsConfig.js Outdated Show resolved Hide resolved

martinvol added 7 commits September 14, 2023 22:23

Added rety for segfault

539d08b

Added more comments to uniswap

c5a6ef6

refactors and comments (may break CI)

94c8dd5

Merge branch 'martinvol/CR10Release' into martinvol/isolate-0.8-artif…

a5ce242

…acts

lint

641ce7b

Changed revert feehandler.ts

b3ae035

Lint, removed console.logs, etc

4f1367e

martinvol mentioned this pull request Sep 15, 2023

Investigate create artifactSet for protocol build #10565

Closed

martinvol and others added 9 commits September 15, 2023 01:38

Adding comments and removing leftovers

1ddf637

Added link to GitHub issue

4209dfb

Added comment

ecf34d2

Workflow syntax

4d93aca

Fix workflow

1c67570

test fixtures

290180e

lint fix

81fce1f

governance test fixes

d73aacf

lint fixes

491a5d0

martinvol merged commit 277107c into martinvol/CR10Release Sep 15, 2023
32 of 37 checks passed

martinvol deleted the martinvol/isolate-0.8-artifacts branch September 15, 2023 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Martinvol/isolate 0.8 artifacts for gasPriceMinimum #10437

Martinvol/isolate 0.8 artifacts for gasPriceMinimum #10437

martinvol commented Jul 26, 2023 •

edited

Loading

socket-security bot commented Jul 26, 2023 •

edited

Loading

martinvol Jul 26, 2023

soloseng left a comment

soloseng Jul 26, 2023

martinvol Aug 9, 2023

m-chrzan Jul 31, 2023

martinvol Aug 9, 2023

martinvol Aug 9, 2023

martinvol Aug 14, 2023

m-chrzan Jul 31, 2023

martinvol Aug 9, 2023

m-chrzan Jul 31, 2023

martinvol Sep 12, 2023

		console.log("this.addresses[library]", this.addresses[library])
		console.log("address", address)

Martinvol/isolate 0.8 artifacts for gasPriceMinimum #10437

Martinvol/isolate 0.8 artifacts for gasPriceMinimum #10437

Conversation

martinvol commented Jul 26, 2023 • edited Loading

Description

Other changes

Tested

Related issues

Backwards compatibility

Documentation

socket-security bot commented Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

soloseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinvol commented Jul 26, 2023 •

edited

Loading

socket-security bot commented Jul 26, 2023 •

edited

Loading