Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade OZ libraries #1707

Merged
merged 10 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 29 additions & 71 deletions contracts/generated/ConstantSupplyERC20/ConstantSupplyERC20.go

Large diffs are not rendered by default.

138 changes: 136 additions & 2 deletions contracts/generated/CrossChainMessenger/CrossChainMessenger.go

Large diffs are not rendered by default.

138 changes: 136 additions & 2 deletions contracts/generated/EthereumBridge/EthereumBridge.go

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

138 changes: 136 additions & 2 deletions contracts/generated/ManagementContract/ManagementContract.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions contracts/generated/MessageBus/MessageBus.go

Large diffs are not rendered by default.

100 changes: 29 additions & 71 deletions contracts/generated/ObsERC20/ObsERC20.go

Large diffs are not rendered by default.

156 changes: 145 additions & 11 deletions contracts/generated/ObscuroBridge/ObscuroBridge.go

Large diffs are not rendered by default.

118 changes: 38 additions & 80 deletions contracts/generated/WrappedERC20/WrappedERC20.go

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion contracts/hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const config: HardhatUserConfig = {
sources: "src"
},
solidity: {
version: "0.8.9",
version: "0.8.20",
settings: {
optimizer: {
enabled: true,
Expand Down Expand Up @@ -55,6 +55,9 @@ const config: HardhatUserConfig = {
warnings : {
'*' : {
default: 'error'
},
'src/testing/**/*': {
default: 'off'
Comment on lines +58 to +60
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning settings for files in the 'src/testing' directory have been turned off. This could potentially hide important warnings during the testing phase. It's recommended to only disable specific warnings that are known to be non-critical after thorough evaluation.

}
}
};
Expand Down
5 changes: 3 additions & 2 deletions contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@
"license": "ISC",
"devDependencies": {
"@nomicfoundation/hardhat-toolbox": "~2.0.0",
"@openzeppelin/contracts": "4.5.0",
"@openzeppelin/hardhat-upgrades": "^1.21.0",
"@solidstate/hardhat-bytecode-exporter": "^1.1.1",
"hardhat": "~2.12.4",
"hardhat": "~2.19.3",
"hardhat-abi-exporter": "^2.10.1",
"hardhat-deploy": "0.11.42",
"node-docker-api": "^1.1.22",
"ts-node": "~10.9.1",
"typescript": "^4.9.4"
},
"dependencies": {
"@openzeppelin/contracts": "^5.0.1",
"@openzeppelin/contracts-upgradeable": "^5.0.1",
"hardhat-ignore-warnings": "^0.2.6"
}
}
8 changes: 5 additions & 3 deletions contracts/src/common/ConstantSupplyERC20.sol
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// SPDX-License-Identifier: Apache 2
pragma solidity >=0.7.0 <0.9.0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider narrowing the pragma solidity version to ^0.8.20 to match the updated Solidity version in hardhat.config.ts and ensure compatibility with the new OpenZeppelin library version.


import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract ConstantSupplyERC20 is ERC20 {

constructor(string memory name, string memory symbol, uint256 initialSupply) ERC20(name, symbol) {
contract ConstantSupplyERC20 is ERC20 {
constructor(string memory name, string memory symbol, uint256 initialSupply)
ERC20(name, symbol)
{
_mint(msg.sender, initialSupply);
}
}
20 changes: 13 additions & 7 deletions contracts/src/management/ManagementContract.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@
pragma solidity >=0.7.0 <0.9.0;

import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";


import "./Structs.sol";
import * as MessageBus from "../messaging/MessageBus.sol";

contract ManagementContract is Ownable, Initializable {
contract ManagementContract is Initializable, OwnableUpgradeable {

using MessageHashUtils for bytes32;
using MessageHashUtils for bytes;

constructor() {
// _disableInitializers(); //todo @siliev - figure out why the solidity compiler cant find this. Perhaps OZ needs a version upgrade?
// _disableInitializers();
_transferOwnership(msg.sender);
}

event LogManagementContractCreated(address messageBusAddress);
Expand Down Expand Up @@ -42,6 +46,7 @@ contract ManagementContract is Ownable, Initializable {
//The messageBus where messages can be sent to Obscuro
MessageBus.IMessageBus public messageBus;
function initialize() public initializer {
__Ownable_init(msg.sender);
lastBatchSeqNo = 0;
messageBus = new MessageBus.MessageBus();
emit LogManagementContractCreated(address(messageBus));
Expand Down Expand Up @@ -113,15 +118,16 @@ contract ManagementContract is Ownable, Initializable {
require(isAggAttested);

if (verifyAttester) {

// the data must be signed with by the correct private key
// signature = f(PubKey, PrivateKey, message)
// address = f(signature, message)
// valid if attesterID = address
bytes32 calculatedHashSigned = ECDSA.toEthSignedMessageHash(abi.encodePacked(attesterID, requesterID, hostAddress, responseSecret));
bytes32 calculatedHashSigned = abi.encodePacked(attesterID, requesterID, hostAddress, responseSecret).toEthSignedMessageHash();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, its in the go code where the signature is generated; We need to append to correct recovery id.

address recoveredAddrSignedCalculated = ECDSA.recover(calculatedHashSigned, attesterSig);

require(recoveredAddrSignedCalculated == attesterID, "calculated address and attesterID dont match");
}
require(recoveredAddrSignedCalculated == attesterID, "calculated address and attesterID dont match");
}

// mark the requesterID aggregator as an attested aggregator and store its host address
attested[requesterID] = true;
Expand Down
3 changes: 3 additions & 0 deletions contracts/src/messaging/MessageBus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import "./Structs.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

contract MessageBus is IMessageBus, Ownable {

constructor() Ownable(msg.sender) {}

function messageFee() internal virtual returns (uint256) {
return 0;
}
Expand Down
16 changes: 13 additions & 3 deletions contracts/test/bridge-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,20 @@ describe("Bridge", function () {
const L1Bridge = await hre.ethers.getContractFactory("ObscuroBridge");
const L2Bridge = await hre.ethers.getContractFactory("EthereumBridge");

const ERC20 = await hre.ethers.getContractFactory("ERC20");
const [owner] = await ethers.getSigners();

const ERC20 = await hre.ethers.getContractFactory("ConstantSupplyERC20", owner);

console.log(`Deploying erc20`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing or commenting out the console.log statement before deployment for cleaner production code.

try {
const erc20 = await ERC20.deploy("XXX", "XXX", 100000);
erc20address = erc20.address;
} catch(err) {
console.error(err);
}


const erc20 = await ERC20.deploy("XXX", "XXX");
erc20address = erc20.address;
console.log(`Deployed erc20`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing or commenting out the console.log statement after deployment for cleaner production code.


busL1 = await MessageBus.deploy();
busL2 = await MessageBus.deploy();
Expand Down
17 changes: 10 additions & 7 deletions go/ethadapter/l1_transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,20 @@ func (l *L1RespondSecretTx) Sign(privateKey *ecdsa.PrivateKey) *L1RespondSecretT
data = append(data, l.HostAddress...)
data = append(data, string(l.Secret)...)

// form the data
msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), string(data))
// hash the data
hashedData := crypto.Keccak256Hash([]byte(msg))
ethereumMessageHash := func(data []byte) []byte {
prefix := fmt.Sprintf("\x19Ethereum Signed Message:\n%d", len(data))
return crypto.Keccak256([]byte(prefix), data)
}

hashedData := ethereumMessageHash(data)
// sign the hash
signedHash, err := crypto.Sign(hashedData.Bytes(), privateKey)
signedHash, err := crypto.Sign(hashedData, privateKey)
if err != nil {
return nil
}
// remove ECDSA recovery id
signedHash = signedHash[:len(signedHash)-1]

// set recovery id to 27; prevent malleable signatures
signedHash[64] += 27
l.AttesterSig = signedHash
return l
}
Expand Down
34 changes: 34 additions & 0 deletions integration/simulation/network/geth_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,40 @@ func StopEth2Network(clients []ethadapter.EthClient, netw eth2network.Eth2Networ
}
}

func InitializeContract(workerClient ethadapter.EthClient, w wallet.Wallet, contractAddress common.Address) (*types.Receipt, error) {
ctr, err := ManagementContract.NewManagementContract(contractAddress, workerClient.EthClient())
if err != nil {
return nil, err
}

opts, err := bind.NewKeyedTransactorWithChainID(w.PrivateKey(), w.ChainID())
if err != nil {
return nil, err
}

tx, err := ctr.Initialize(opts)
if err != nil {
return nil, err
}
w.SetNonce(w.GetNonce())

var start time.Time
var receipt *types.Receipt
// todo (@matt) these timings should be driven by the L2 batch times and L1 block times
for start = time.Now(); time.Since(start) < 80*time.Second; time.Sleep(2 * time.Second) {
receipt, err = workerClient.TransactionReceipt(tx.Hash())
if err == nil && receipt != nil {
if receipt.Status != types.ReceiptStatusSuccessful {
return nil, errors.New("unable to initialize contract")
}
testlog.Logger().Info("Contract initialized")
return receipt, nil
}
}

return receipt, nil
}

// DeployContract returns receipt of deployment
// todo (@matt) - this should live somewhere else
func DeployContract(workerClient ethadapter.EthClient, w wallet.Wallet, contractBytes []byte) (*types.Receipt, error) {
Expand Down
19 changes: 19 additions & 0 deletions integration/smartcontract/smartcontracts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,31 @@ func TestManagementContract(t *testing.T) {
if err != nil {
panic(err)
}

nonce, err := client.Nonce(w.Address())
if err != nil {
t.Error(err)
}

w.SetNonce(nonce)
// deploy the same contract to a new address
receipt, err := network.DeployContract(client, w, bytecode)
if err != nil {
t.Error(err)
}

_, err = network.InitializeContract(client, w, receipt.ContractAddress)
if err != nil {
t.Error(err)
}

nonce, err = client.Nonce(w.Address())
if err != nil {
t.Error(err)
}

w.SetNonce(nonce)

// run the test using the new contract, but same wallet
test(t,
newDebugMgmtContractLib(receipt.ContractAddress, client.EthClient(),
Expand Down
Loading