Storing previous randomness values #1197

mrsmkl · 2019-10-03T10:33:19Z

Description

Just stores the randomness value to a mapping. If it is too much overhead to store it for all blocks, it can be easily changed to just store N previous blocks.

Tested

I didn't see any tests related to randomness.

Other changes

Related issues

Fixes Developers SBAT have access to randomness of prior blocks #1181

Backwards compatibility

nambrot · 2019-10-04T10:18:39Z

Thanks for the PR @mrsmkl !

We are currently still discussing the specific change, but I can tell you that we can't store the whole history for sure. Ill update here once that is the case!

asaj · 2019-10-08T17:57:27Z

I didn't see any tests related to randomness.

Would you mind adding some? I suspect we don't have them because of the require(msg.sender == address(0)) but this is easily worked around.

We can define an internal function that has all of the business logic, and then expose it in an external function that just requires msg.sender == address(0) and then calls the internal function.

Then, for testing, we can create contracts/identity/test/RandomTest.sol that inherits from Random and exposes the internal function.

Here's an example for Validators, which will soon have the same issue:
https://github.com/celo-org/celo-monorepo/blob/asaj/pos-2/packages/protocol/contracts/governance/Validators.sol#L439
https://github.com/celo-org/celo-monorepo/blob/asaj/pos-2/packages/protocol/contracts/governance/test/ValidatorsTest.sol
https://github.com/celo-org/celo-monorepo/blob/asaj/pos-2/packages/protocol/test/governance/validators.ts#L1352

codecov · 2019-10-09T13:05:59Z

Codecov Report

Merging #1197 into master will increase coverage by 1.76%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #1197      +/-   ##
=========================================
+ Coverage   65.33%   67.1%   +1.76%     
=========================================
  Files         271     266       -5     
  Lines        8184    7776     -408     
  Branches      570     444     -126     
=========================================
- Hits         5347    5218     -129     
+ Misses       2716    2458     -258     
+ Partials      121     100      -21

Flag	Coverage Δ
#mobile	`67.1% <ø> (+1.76%)`	⬆️

Impacted Files	Coverage Δ
packages/mobile/src/exchange/actions.ts	`28.15% <0%> (-71.85%)`	⬇️
packages/mobile/src/pincode/PincodeCache.ts	`37.5% <0%> (-25%)`	⬇️
packages/mobile/src/firebase/firebase.ts	`20.89% <0%> (-21.33%)`	⬇️
packages/mobile/src/backup/utils.ts	`70.73% <0%> (-11.63%)`	⬇️
packages/mobile/src/exchange/reducer.ts	`60% <0%> (-6.67%)`	⬇️
packages/mobile/src/utils/AppRestart.ts	`50% <0%> (-6.25%)`	⬇️
packages/mobile/src/import/reducer.ts	`37.5% <0%> (-5.36%)`	⬇️
packages/mobile/src/firebase/notifications.ts	`40.54% <0%> (-5.18%)`	⬇️
packages/mobile/src/backup/BackupComplete.tsx	`81.25% <0%> (-4.47%)`	⬇️
packages/mobile/src/redux/store.ts	`96% <0%> (-4%)`	⬇️
... and 80 more

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 947e237...a488cd5. Read the comment docs.

packages/protocol/contracts/identity/Random.sol

asaj · 2019-10-10T23:33:13Z

packages/protocol/contracts/identity/Random.sol

@@ -13,7 +13,18 @@ contract Random is IRandom {

  bytes32 public _random;


Is this now redundant with history?

packages/protocol/contracts/identity/Random.sol

packages/protocol/test/identity/random.ts

packages/protocol/contracts/identity/Random.sol

asaj · 2019-10-10T23:57:52Z

For some reason the end-to-end transfer tests appear to be failing..

…mness-history-1181

…repo into randomness-history-1181

asaj

Just a few nits

packages/protocol/contracts/identity/Random.sol

packages/protocol/contracts/identity/test/TestRandom.sol

packages/protocol/test/identity/random.ts

packages/protocol/contracts/identity/Random.sol

packages/protocol/migrationsConfig.js

asaj · 2019-10-18T16:45:48Z

README-dev.md

-  2. Exception to (1) are packages that represent a GAE/firebase app which must use the last published version.
-  3. To differentiate published vs unpublished version. Master version (in package.json) must end with suffix `-dev` and should not be published.
-  4. If a developer want to publish a version; then after publishing it needs to set master version to next `-dev` version and change all package.json that require on it.
+1.  All packages must use **master version** of sibling packages.


Were the changes to the readmes intentional? If not, please revert.

I wonder where this came from...

nambrot · 2019-10-18T22:01:09Z

Let's hold off on merging this until we cut for Alfajores deploy since this is breaking mobile clients

…repo into aaronmgdr/invite-page * 'aaronmgdr/invite-page' of github.com:celo-org/celo-monorepo: (63 commits) Fix compile error during local clean build (#1506) Update to RN 61 and AndroidX (#1343) Set usage of shuffled round robin in the genesis block (#1464) Add spanish backup key for backup key flow (#1500) Fix sync tests by pulling genesis block to determine epoch length (#1504) [Wallet] fix missing full name error alert (#1496) [Wallet + Verification Pool] Add details about generating an app-signature (#1482) Deploy celo's image of ethstats (#1421) Storing previous randomness values (#1197) [Wallet] Fix wei invite bug (#1489) Point all packages to latest ganache-cli master (#1488) Point end-to-end tests back to master (#1469) [Wallet] Migrate app view functions to contractkit (#1381) [Wallet] Add script to translate locale strings (#1485) [Wallet] Update wallet celo client version and add missing translations for backup flow (#1483) [Wallet] Hotfix local currency (#1481) [Wallet] Remove QR debouncing to improve responsiveness (#1480) [Wallet] Upgrade app version to v1.5.1 (#1463) Update governance end-to-end tests to work with changed precompile (#1476) Fixes key_placer.sh when encrypting files (#1465) ...

store previous randomness values

bc4ee67

mrsmkl requested review from asaj and m-chrzan as code owners October 3, 2019 10:33

mrsmkl assigned nambrot Oct 3, 2019

limited size of history

d75a4e6

mrsmkl added 2 commits October 9, 2019 15:03

this won't work

5ab9df2

added first tests

e5b1794

cleanup

876259c

nambrot assigned asaj and unassigned nambrot Oct 10, 2019

asaj reviewed Oct 10, 2019

View reviewed changes

asaj assigned mrsmkl and unassigned asaj Oct 10, 2019

mrsmkl added 4 commits October 11, 2019 08:24

Merge branch 'master' of github.com:celo-org/celo-monorepo into rando…

504de69

…mness-history-1181

review changes

5ffe716

fixing lint

0fb543a

Merge branch 'master' into randomness-history-1181

71324ca

nambrot mentioned this pull request Oct 14, 2019

Add selectIssuers Transaction #1327

Merged

3 tasks

mrsmkl assigned asaj and nambrot and unassigned mrsmkl Oct 15, 2019

mrsmkl added 5 commits October 15, 2019 20:16

remove _random, checking if something is broken

faf1ac3

Merge branch 'master' of github.com:celo-org/celo-monorepo into rando…

f059b9d

…mness-history-1181

Merge branch 'randomness-history-1181' of github.com:mrsmkl/celo-mono…

c370059

…repo into randomness-history-1181

added require errors

4cadc87

fixed lint

dd89be9

mrsmkl added 4 commits October 15, 2019 20:41

bump

a4045cf

bump

2862f33

oops

346a220

fixed tests

02a3c25

asaj reviewed Oct 17, 2019

View reviewed changes

asaj assigned mrsmkl and unassigned asaj Oct 17, 2019

mrsmkl added 2 commits October 18, 2019 06:13

merge

0ead27d

added comments

eabd053

mrsmkl requested review from cmcewen and jmrossy as code owners October 18, 2019 03:58

fixing lint

911e8a9

mrsmkl assigned asaj and unassigned mrsmkl Oct 18, 2019

trying to remove some flakiness

a4af20b

asaj approved these changes Oct 18, 2019

View reviewed changes

asaj assigned mrsmkl and unassigned asaj Oct 18, 2019

mrsmkl added 2 commits October 18, 2019 22:19

reverting changes

92eaeb6

trying to reset

ec33e52

mrsmkl added the automerge Have PR merge automatically when checks pass label Oct 18, 2019

Merge branch 'master' into randomness-history-1181

388ba22

nambrot removed the automerge Have PR merge automatically when checks pass label Oct 18, 2019

asaj self-assigned this Oct 25, 2019

nambrot and others added 2 commits October 25, 2019 12:25

Merge branch 'master' into randomness-history-1181

1c76973

Merge branch 'master' into randomness-history-1181

a488cd5

asaj added the automerge Have PR merge automatically when checks pass label Oct 25, 2019

celo-ci-bot-user merged commit e153a29 into celo-org:master Oct 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storing previous randomness values #1197

Storing previous randomness values #1197

mrsmkl commented Oct 3, 2019

nambrot commented Oct 4, 2019

asaj commented Oct 8, 2019 •

edited

Loading

codecov bot commented Oct 9, 2019 •

edited

Loading

asaj Oct 10, 2019

asaj commented Oct 10, 2019

asaj left a comment

asaj Oct 18, 2019

mrsmkl Oct 18, 2019

nambrot commented Oct 18, 2019

		@@ -13,7 +13,18 @@ contract Random is IRandom {

		bytes32 public _random;

Storing previous randomness values #1197

Storing previous randomness values #1197

Conversation

mrsmkl commented Oct 3, 2019

Description

Tested

Other changes

Related issues

Backwards compatibility

nambrot commented Oct 4, 2019

asaj commented Oct 8, 2019 • edited Loading

codecov bot commented Oct 9, 2019 • edited Loading

Codecov Report

asaj Oct 10, 2019

Choose a reason for hiding this comment

asaj commented Oct 10, 2019

asaj left a comment

Choose a reason for hiding this comment

asaj Oct 18, 2019

Choose a reason for hiding this comment

mrsmkl Oct 18, 2019

Choose a reason for hiding this comment

nambrot commented Oct 18, 2019

asaj commented Oct 8, 2019 •

edited

Loading

codecov bot commented Oct 9, 2019 •

edited

Loading