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.

Sign up for GitHub

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

Jump to bottom

SemaphoreViem implemented #364

Closed
wants to merge 901 commits into from
Closed

SemaphoreViem implemented #364

wants to merge 901 commits into from

Conversation

inaridiy
Copy link

@inaridiy inaridiy commented Sep 22, 2023 •
edited by cedoor
Loading

Description

SemaphoreViem is implemented.

Related Issue

Closes #343

Does this introduce a breaking change?

  • Yes
  • No

Other information

cedoor and others added 30 commits February 15, 2023 20:40
@cedoor
ci(release): add new workflow for automatic releases
a4fcb36
@cedoor
chore(semaphore): v3.1.0
243c505
@cedoor
chore: update lockfile
4f49736
@cedoor
docs(subgraph): update readme file
6e036c0
@cedoor
chore(release): create yarn patch to update changelogithub format
739af09 
re semaphore-protocol#252
@cedoor
Merge pull request semaphore-protocol#253 from semaphore-protocol/cho…
d2b9597 
…re/release

Yarn patch to change `changelogithub` output
@shiyingt
add hardhat task to deploy semaphore verifier only
972790d
@cedoor
chore(yarn): add yarn plugin for versioning
a5df125 
re semaphore-protocol#254
@cedoor
chore: add new workspace package
9ef675f 
re semaphore-protocol#254
@cedoor
chore: update lockfile
0b1519e 
re semaphore-protocol#254
@cedoor
chore: add new scripts to bump versions & publish npm pkgs
257080e 
re semaphore-protocol#254
@cedoor
style: format code with prettier
5c402f3
@cedoor
Merge pull request semaphore-protocol#255 from semaphore-protocol/cho…
1bb2982 
…re/bump-version

New NPM scripts to bump versions and publish NPM packages
@shiyingt
fix hardhat task
3105a0c
@shiyingt
fix hardhat task
6987686
@cedoor
chore: update discord links
bd06422
@shiyingt
run prettier write and eslint
2ddbc51
@cedoor
docs(hardhat): update readme description
865421a
@shiyingt
revert all changes
b51f1d7
@shiyingt
add hardhat task to deploy verifier
934e4b2
@cedoor
Merge pull request semaphore-protocol#257 from shiyingt/feat/deploy-v…
76146ed 
…erifier-hardhat

add hardhat task to deploy SemaphoreVerifier.sol
@cedoor
refactor(hardhat): update readme and set correct task name
25e2442
@chancehudson
feat(identity): prepend hex strings with 0x
3c359e9
@cedoor
Merge pull request semaphore-protocol#261 from vimwitch/json-prepend-0x
5a99792 
feat(identity): prepend hex strings with 0x
@chancehudson
feat(identity): use sha512 to extract more entropy from message
ed63519
@chancehudson
chore: run prettier
d5d0114
@cedoor
chore(subgraph): remove subgraph package
b040c42 
re semaphore-protocol#256
@cedoor
Merge pull request semaphore-protocol#262 from vimwitch/identity-entropy
95e4a4e 
feat(identity): use sha512 to extract more entropy from message
@cedoor
feat(data): create new data package
fc2f648 
This new package allows devs to fetch on-chain data by using a Semaphore subgraph or the Ethers
library.

BREAKING CHANGE: The code of the old `@semaphore-protocol/subgraph` package has been moved to the
`@semaphore-protocol/data` package.

re semaphore-protocol#256
@cedoor
chore: remove types folder from prettier ignore file
925cc64
@inaridiy inaridiy mentioned this pull request Sep 22, 2023
Open
@inaridiy
Copy link
Author

Worse, I forgot to write the test. I'll go write one soon.

@vplasencia
Copy link
Member

Hey @inaridiy! Don't worry, take your time. Thank you so much.

@inaridiy
Copy link
Author

@vplasencia Accomplished! I have confirmed a successful build and test!

cedoor
cedoor requested changes Sep 27, 2023
View reviewed changes
packages/data/src/types/index.ts Show resolved Hide resolved
packages/data/src/viem.ts Outdated Show resolved Hide resolved
packages/data/src/viem.ts
* @param network Chain object of viem.
* @param options Viem options.
*/
constructor(network: Chain = sepolia, options: ViemOptions = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice for devs to have a way to pass a valid chain without installing vite separately and importing viem/chains. The type here can be similar to EthersNetwork (i.e. strings), and the constructor could then use the right chain object based on that string. It would also be more consistent with SemaphoreEthers.

Copy link
Author

Choose a reason for hiding this comment

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

I think the current implementation is better here, given its affinity with the Viem ecosystem.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Do you think it makes sense to export those chains' objects in this package too?

Copy link
Author

Choose a reason for hiding this comment

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

It certainly makes sense, and I find it a very nice contrast to SemaphoreEthers.

packages/data/src/viem.ts Outdated Show resolved Hide resolved
@inaridiy
Copy link
Author

inaridiy commented Oct 2, 2023

Thanks for the review. I will fix it now.

@inaridiy
Copy link
Author

inaridiy commented Oct 2, 2023

I have made some corrections and added some comments

@cedoor
Copy link
Member

cedoor commented Oct 4, 2023

I have made some corrections and added some comments

Great, it looks like there's a small error in the API test.

@vplasencia vplasencia requested a review from a team as a code owner July 25, 2024 10:49
@cedoor
Copy link
Member

cedoor commented Sep 2, 2024

Hey @inaridiy, after releasing Semaphore V4 many conflicts have come out. It may make sense to open another clean PR. Do you still want to work on this?

@cedoor cedoor mentioned this pull request Sep 2, 2024
Open
@inaridiy
Copy link
Author

inaridiy commented Sep 2, 2024

@cedoor Thank you for the follow-up. I apologize for my long inactivity on this PR. Given the conflicts with Semaphore V4 and my inability to dedicate proper time, I agree it's best to close this PR.
I still believe in the value of Viem integration, but it deserves a fresh start. If there's interest in the future, I'd be happy to contribute again or support others taking on this task.
Thank you for your understanding. Please feel free to close this PR. I appreciate the opportunity and will keep an eye out for future ways to contribute to Semaphore.

@cedoor
Copy link
Member

cedoor commented Sep 3, 2024 •
edited
Loading

Hi @inaridiy, thanks for your quick reply and the work you've done here.

I'll close this PR but a gitPOAP will still be awarded to you :)

@cedoor
Copy link
Member

cedoor commented Sep 3, 2024

@inaridiy @gitpoap-bot

@gitpoap-bot gitpoap-bot
Copy link

gitpoap-bot bot commented Sep 3, 2024

Congrats, @inaridiy ! You've earned a GitPOAP for your contribution!

GitPOAP: 2023 Semaphore Contributor:

GitPOAP: 2023 Semaphore Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@cedoor cedoor closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedoor cedoor cedoor requested changes

@vplasencia vplasencia Awaiting requested review from vplasencia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create lib that uses Viem
10 participants
@inaridiy @vplasencia @cedoor @shiyingt @chancehudson @zkzoomer @dbrans @mmqxyz @wslyvh @aguzmant103