vacp2p / rln-contract

RLN contract
Apache License 2.0
13 stars 6 forks source link

chore: use deployed block number #27

Closed rymnc closed 1 year ago

rymnc commented 1 year ago

The deployedBlockNumber variable is used so that clients can directly start listening to events from this block

github-actions[bot] commented 1 year ago

LCOV of commit 520d92c during CI #36

Summary coverage rate:
  lines......: 84.0% (497 of 592 lines)
  functions..: 50.0% (13 of 26 functions)
  branches...: 52.5% (21 of 40 branches)

Files changed coverage rate:
                              |Lines       |Functions  |Branches    
  Filename                    |Rate     Num|Rate    Num|Rate     Num
  ==================================================================
  contracts/RlnBase.sol       |97.1%     34|87.5%     8| 100%     18
github-actions[bot] commented 1 year ago

LCOV of commit b043264 during CI #37

Summary coverage rate:
  lines......: 84.0% (497 of 592 lines)
  functions..: 50.0% (13 of 26 functions)
  branches...: 52.5% (21 of 40 branches)

Files changed coverage rate:
                              |Lines       |Functions  |Branches    
  Filename                    |Rate     Num|Rate    Num|Rate     Num
  ==================================================================
  contracts/RlnBase.sol       |97.1%     34|87.5%     8| 100%     18
github-actions[bot] commented 1 year ago

LCOV of commit 54af563 during CI #38

Summary coverage rate:
  lines......: 84.0% (497 of 592 lines)
  functions..: 50.0% (13 of 26 functions)
  branches...: 52.5% (21 of 40 branches)

Files changed coverage rate:
                              |Lines       |Functions  |Branches    
  Filename                    |Rate     Num|Rate    Num|Rate     Num
  ==================================================================
  contracts/RlnBase.sol       |97.1%     34|87.5%     8| 100%     18
rymnc commented 1 year ago

@0x-r4bbit what do you think about this pattern? is there a better way than using a storage slot to store a block number?

0x-r4bbit commented 1 year ago

@rymnc we can probably use a smaller uint type so we don't take up an entire slot for the block number. As of writing this comment, ethereum's block number is 17939927 which easily fits into uint32 but would be too big for uint16.

Only thing I'm unsure about is, what if we run into a point where the blocknumber exceeds uint32 (that number is very large so not gonna happen any time soon). Not sure if this is a case we need to handle.

rymnc commented 1 year ago

@0x-r4bbit nice suggestion, using a bigger uint doesn't really matter if we deploy before it gets bigger than uint32 lol (which we should)

rymnc commented 1 year ago

Addressed in 7ad7915

github-actions[bot] commented 1 year ago

LCOV of commit 7ad7915 during CI #39

Summary coverage rate:
  lines......: 84.0% (497 of 592 lines)
  functions..: 50.0% (13 of 26 functions)
  branches...: 52.5% (21 of 40 branches)

Files changed coverage rate:
                              |Lines       |Functions  |Branches    
  Filename                    |Rate     Num|Rate    Num|Rate     Num
  ==================================================================
  contracts/RlnBase.sol       |97.1%     34|87.5%     8| 100%     18
0x-r4bbit commented 1 year ago

nice suggestion

@rymnc to be very honest, I didn't even think about it until you explicitly asked for it lol

using a bigger uint doesn't really matter if we deploy before it gets bigger than uint32

Right!