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

fix(rln-relay): sync from a recent block number, not 0 #1859

Closed
wants to merge 3 commits into from

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Jul 25, 2023

!WIP!

  • fix(rln-relay): sync from a recent block number, not 0

Description

Changes

  • ...
  • ...

@rymnc
Copy link
Contributor Author

rymnc commented Jul 31, 2023

This PR is no longer required since it was fixed with the decay introduction in #1858

@rymnc rymnc closed this Jul 31, 2023
@rymnc rymnc deleted the start-sync-from-deployed-block branch July 31, 2023 05:10
@alrevuelta
Copy link
Contributor

can see its closed but out of curiosity. which block is it used to sync the registration events? from 0?

@rymnc
Copy link
Contributor Author

rymnc commented Aug 7, 2023

can see its closed but out of curiosity. which block is it used to sync the registration events? from 0?

Yes, as of now, its 0 - but when waku-org/waku-rln-contract is a submodule, we will fetch from the block number in which the contract was deployed

@alrevuelta
Copy link
Contributor

@rymnc Leaving the idea in case you find it useful. afaik in solidity you can access the block block.number. So in the constructor that is called on deployment, just store block.number and then add a getter to it.

So then we can just getDeployedBlockNumber() and sync from there.

Is that what you had in mind by "we will fetch from the block number in which the contract was deployed" ?

@rymnc
Copy link
Contributor Author

rymnc commented Aug 8, 2023

Sure, we can add block.number in the constructor, it's unnecessary storage though. (open to this solution)
by
"we will fetch from the block number in which the contract was deployed", i meant from the deployment stored in the deployments directory in waku-org/waku-rln-contract

@alrevuelta
Copy link
Contributor

it's unnecessary storage though

shouldn't be expensive, just one variable? isnt it neglectable?

i meant from the deployment stored in the deployments directory in waku-org/waku-rln-contract

but how will people access it? manually? or how can this be automated?

@rymnc
Copy link
Contributor Author

rymnc commented Aug 8, 2023

shouldn't be expensive, just one variable? isnt it neglectable?

agree, it can be neglected - I'll make the changes upstream for this

but how will people access it? manually? or how can this be automated?

since it is a submodule in nwaku, it will be automated - check #1884, where its automatically fetched

@alrevuelta
Copy link
Contributor

since it is a submodule in nwaku, it will be automated - check #1884, where its automatically fetched

ah i see. but then if someone deploys its own rln contract, this wont work, and we would need a flag to specify it and override the default?

@rymnc
Copy link
Contributor Author

rymnc commented Aug 9, 2023

since it is a submodule in nwaku, it will be automated - check #1884, where its automatically fetched

ah i see. but then if someone deploys its own rln contract, this wont work, and we would need a flag to specify it and override the default?

Correct - I will use your approach moving forward, but there still may be some merit in using waku-rln-contract as a submodule, wdyt?

@alrevuelta
Copy link
Contributor

alrevuelta commented Aug 9, 2023

but there still may be some merit in using waku-rln-contract as a submodule, wdyt?

having the ABI is needed i guess? so that would be a merit. is it hardcoded now?

@rymnc
Copy link
Contributor Author

rymnc commented Aug 9, 2023

nim-web3 doesn't need the abi I believe ~ it wouldn't be too useful for nwaku except for using the deployedBytecode from the artifacts (but thats only for tests)

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.

2 participants