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

Limechain/16xx #1982

Merged
merged 14 commits into from
Dec 27, 2019
Merged

Limechain/16xx #1982

merged 14 commits into from
Dec 27, 2019

Conversation

packages/protocol/contracts/common/UsingRegistry.sol Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts.old Outdated Show resolved Hide resolved
packages/protocol/contracts/common/MultiSig.sol Outdated Show resolved Hide resolved
@asaj asaj assigned BatiGencho and unassigned asaj Dec 3, 2019
@codecov
Copy link

codecov bot commented Dec 5, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1982    +/-   ##
========================================
  Coverage   74.66%   74.66%            
========================================
  Files         274      274            
  Lines        7732     7732            
  Branches      701      985   +284     
========================================
  Hits         5773     5773            
  Misses       1847     1847            
  Partials      112      112
Flag Coverage Δ
#mobile 74.66% <ø> (ø) ⬆️

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 a9a5789...a5adeec. Read the comment docs.

@asaj
Copy link
Contributor

asaj commented Dec 6, 2019

Please include any additional tickets you've addressed in the PR description

Copy link
Contributor

@asaj asaj left a comment

Choose a reason for hiding this comment

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

Thanks for reviving the multisig tests!

packages/protocol/contracts/common/GasPriceMinimum.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/common/GasPriceMinimum.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/common/UsingRegistry.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/common/UsingRegistry.sol Outdated Show resolved Hide resolved
packages/protocol/migrationsConfig.js Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
@asaj asaj removed their assignment Dec 6, 2019
@asaj asaj removed their assignment Dec 10, 2019
@asaj asaj added the audit label Dec 20, 2019
Copy link
Contributor

@asaj asaj left a comment

Choose a reason for hiding this comment

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

Looks great! Just some minor comments on the multisig tests

packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
assert.isTrue(await multiSig.isOwner(accounts[2]))
})

it('should not allow an external account to add an owner', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this test instead be calling addOwner directly rather than submitTransaction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

packages/protocol/test/common/multisig.ts Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Outdated Show resolved Hide resolved
packages/protocol/test/common/multisig.ts Show resolved Hide resolved
@asaj asaj removed their assignment Dec 21, 2019
Copy link
Contributor

@mcortesi mcortesi left a comment

Choose a reason for hiding this comment

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

LGTM

@mcortesi mcortesi added the automerge Have PR merge automatically when checks pass label Dec 27, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit ea91f52 into celo-org:master Dec 27, 2019
@mrsmkl mrsmkl mentioned this pull request Dec 27, 2019
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (26 commits)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  [Wallet] Fix exchange input on iOS and feed item display (#2319)
  ...

# Conflicts:
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (35 commits)
  Remove rep sentence from brand kit page (#2350)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment