feat: refine work with membership info and other meta information #2341
Conversation
size-limit report 📦
|
danisharora099
force-pushed
the
feat/rln-improvements
branch
5 times, most recently
from
197ba46 to
d0da8f8
Compare
danisharora099
force-pushed
the
feat/rln-improvements
branch
from
a824fff to
08af18e
Compare
|
@danisharora099 can you create an issue capturing this problem(s)? and can you, please, make PR name more informative? |
| } | ||
|
|
||
| public async withdraw(token: string, holder: string): Promise<void> { | ||
| public async withdraw(token: string, from: string): Promise<void> { |
There was a problem hiding this comment.
if from wallet address? if so, I would name it like this
| ) { | ||
| private async validateRateLimit(rateLimit: number): Promise<void> { | ||
| const [minRate, maxRate] = await Promise.all([ | ||
| this.contract.minMembershipRateLimit(), |
There was a problem hiding this comment.
this is something that doesn't change during lifecycle of the contract once it is deployed, I believe
if I am not wrong, then you should fetch this value when contract instance gets instantiated
for that I would recommend using a static method similarly how it is done here
https://github.com/waku-org/js-rln/blob/f6d5deb8cb51602b2b544de5de5b88de96e11aef/src/contract/rln_contract.ts#L56
after that this method becomes sync again
There was a problem hiding this comment.
Done here: 10aadbe
| } | ||
|
|
||
| // Initialize members and subscriptions | ||
| this.fetchMembers() |
There was a problem hiding this comment.
I would recommend to improve it even further and:
- use similar approach to https://github.com/waku-org/js-rln/blob/f6d5deb8cb51602b2b544de5de5b88de96e11aef/src/contract/rln_contract.ts#L56 for both contract implementations;
- inject an instance of base contract into specific contract and not use
extend
|
@weboko can you please highlight which comments are blocking comments, and which are nits that aren't as important? |
danisharora099
force-pushed
the
feat/rln-improvements
branch
from
a7f52e4 to
6d86b6f
Compare
weboko
changed the title
Problem / Description
There are a few problems with the current adapter:
idCommitmentis a stringified hexgetMembershipInfodoesn't return all available membership info fieldsvalidateRateLimitvalidates the upper and lower bounds defined in the adapter, instead of the contractSolution
bigintforidCommitmentinstead ofstringgetMembershipInfoto fetch, and return, ALL available dataNotes
Checklist