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

Feature #909 proxy delegatecall #1152

Merged
merged 46 commits into from
Oct 7, 2019
Merged

Feature #909 proxy delegatecall #1152

merged 46 commits into from
Oct 7, 2019

Conversation

aaitor
Copy link
Contributor

@aaitor aaitor commented Oct 1, 2019

Description

The intention of this PR is to validate if an address is a contract or EOA before calling the delegatecall

Tested

Test added in proxy.ts calling _setAndInitializeImplementation with a EOA address.

Other changes

The isContract function shouldn't be called from a constructor. Calling this function from a constructor will return 0 independently if the address given as parameter is a contract or EOA

Related issues

Fixes #909 , #910, #911

Backwards compatibility

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

r00ted42 and others added 30 commits September 12, 2019 18:44
Bringing celo-monorepo:master
@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #1152 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1152   +/-   ##
=======================================
  Coverage   66.47%   66.47%           
=======================================
  Files         262      262           
  Lines        7648     7648           
  Branches      441      509   +68     
=======================================
  Hits         5084     5084           
+ Misses       2464     2462    -2     
- Partials      100      102    +2
Flag Coverage Δ
#mobile 66.47% <ø> (ø) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/utils/formatting.ts 89.74% <0%> (ø) ⬆️
packages/mobile/src/identity/reducer.ts 41.66% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cd3614...b9ec124. Read the comment docs.

@asaj asaj added the automerge Have PR merge automatically when checks pass label Oct 7, 2019
@ashishb ashishb merged commit 78afc93 into celo-org:master Oct 7, 2019
asaj added a commit that referenced this pull request Oct 8, 2019
asaj added a commit that referenced this pull request Oct 8, 2019
aaronmgdr added a commit that referenced this pull request Oct 8, 2019
* master: (35 commits)
  [Wallet] Fix top of emojis cut off in the activity feed (#1243)
  Adding a contract to store minimum required client version (#1081)
  Revert "Feature #909 proxy delegatecall (#1152)" (#1241)
  Use ContractKit to get addresses for Blockchain API (#1175)
  Feature #909 proxy delegatecall (#1152)
  Fix Faucet done message (#1217)
  Updated SETUP.md with new yarn process (#1224)
  Adding `increaseAllowance` and `decreaseAllowance` methods (#1196)
  extracting function signatures (#1061)
  Fix integration hardcode (#1208)
  Fixing flaky governance test (#1155)
  Restore CI branch (#1223)
  [wallet] e2e back to green (#1210)
  [Wallet] Implement new import wallet flow designs (#1209)
  [Wallet] Fix disable conditions for butons on Enter Invite screen (#1214)
  [protocol] Rename infrastructureFraction to proposerFraction (#1174)
  [ck] Proper promise treatment to avoid UnhandledPromises (#1219)
  [ck] Transform StableToken parameters from fixidity format (#1218)
  [wallet]Store encrypted local signing key (#1188)
  2019-10-03 alfajores deployment (#1200)
  ...
ashishb added a commit that referenced this pull request Oct 8, 2019
Earlier, it would take the first page of commits from the GitHub's PR
commits page.
Now, it follows the next link url to fetch all commits.

Tested with

```
CIRCLE_PROJECT_USERNAME=celo-org CIRCLE_PROJECT_REPONAME=celo-monorepo CIRCLE_PULL_REQUEST="#1152" ./scripts/ci_check_if_test_should_run_v2.sh $PWD/packages/protocol
```

Earlier, this would check (First) 30 commits. Now, it checks all 46
commits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Developers SBAT check contract code existence in Proxy contract
6 participants