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

Implement proxy contract #66

Merged
merged 29 commits into from
Sep 6, 2024
Merged

Implement proxy contract #66

merged 29 commits into from
Sep 6, 2024

Conversation

karim-en
Copy link
Collaborator

@karim-en karim-en commented Apr 9, 2024

Motivation

We need to extend eth-connector to be able to do the following:

  • Pause the contract with one click without multisig.
  • Restrict deposit for non-legitimate addresses to near after the integration with Aethos.
  • Add the possibility to finalize the pre-migration transactions.

Implementation

eth-connector is not upgradeable so direct change of implementation is not possible. However, it extends AdminControlled. According to AdminControlled implementation, the admin has control over the contract's storage and can pause contract functionality for everyone but themselves. Therefore it was decided that the simplest way to extend eth-connector's functionality is to:

  • Create a new proxy contract
  • Make it admin of eth-connector
  • Pause all eth-connector functions for everyone but the admin
  • Route new requests through the proxy contract

As a result we can get all 3 of the required upgrades:

  • One-click pauseability is easily achieved by implementing it on the proxy contract.
  • Same goes for any logic related to restricting deposits.
  • To finalize withdrawals for the old proof producer (pre-migration) we use admin's ability to override storage slots on eth-connector. We override it to the old producer before making a withdrawal and then return to the original value at the end of the transaction.

TODO?

  • move SelectivePausableUpgradable.sol to the shared repo

@karim-en karim-en requested review from olga24912 and kisialiou April 9, 2024 00:20
@karim-en karim-en requested a review from kiseln April 9, 2024 14:57
eth-custodian/contracts/EthCustodianProxy.sol Outdated Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Outdated Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Outdated Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Outdated Show resolved Hide resolved
@kiseln kiseln marked this pull request as ready for review April 22, 2024 16:07
eth-custodian/test/EthCustodianProxy.js Outdated Show resolved Hide resolved
eth-custodian/test/EthCustodianProxy.js Outdated Show resolved Hide resolved
eth-custodian/test/EthCustodianProxy.js Show resolved Hide resolved
eth-custodian/test/EthCustodianProxy.js Show resolved Hide resolved
eth-custodian/test/EthCustodianProxy.js Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@karim-en karim-en left a comment

Choose a reason for hiding this comment

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

@kiseln Please add a generic method (Dynamic) to call any impl method.
Please also add a test where you change the admin address over it.

@kiseln
Copy link
Contributor

kiseln commented Apr 29, 2024

@kiseln Please add a generic method (Dynamic) to call any impl method. Please also add a test where you change the admin address over it.

See ec064b5

olga24912
olga24912 previously approved these changes May 8, 2024
eth-custodian/contracts/EthCustodianProxy.sol Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Outdated Show resolved Hide resolved
eth-custodian/test/EthCustodianProxy.js Outdated Show resolved Hide resolved
@kiseln kiseln requested a review from olga24912 May 15, 2024 17:22
olga24912
olga24912 previously approved these changes May 29, 2024
eth-custodian/test/EthCustodianProxy.js Outdated Show resolved Hide resolved
@karim-en karim-en requested a review from olga24912 June 21, 2024 14:21
kiseln
kiseln previously approved these changes Jul 3, 2024
eth-custodian/contracts/EthCustodianProxy.sol Show resolved Hide resolved
eth-custodian/contracts/EthCustodianProxy.sol Show resolved Hide resolved
eth-custodian/contracts/ProofKeeperGap.sol Outdated Show resolved Hide resolved
@kiseln kiseln self-requested a review July 3, 2024 17:21
karim-en and others added 4 commits July 3, 2024 20:46
* draft getBlockHeightFromProof

* debug

* fix extract block height from proof

* fix tests and contract

* separate BlockHeightFromProofExtractor

* add new test

* fix tests

* fix tests

* add checkPreMigration flag

* add test with switched off preMigration check

* add comments about proof structure

* add comment for withdraw function

* checkPreMigration -> receiptBlockHeight
@karim-en karim-en merged commit 5d59591 into master Sep 6, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants