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

Scroll Ecosystem Update #658

Merged

Conversation

dghelm
Copy link
Contributor

@dghelm dghelm commented Nov 15, 2023

Run from internal tracking of hackers and ecosystem projects. This is our first time making a PR from output of this script and datasource, so we'd appreciate any feedback moving forwards.

Subsequent diffs should be easier to read, as the script uses alphabetical ordering.

Copy link
Contributor

@eherrerosj eherrerosj left a comment

Choose a reason for hiding this comment

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

This is off to a good start! There's some work that needs to be done I'm seeing:

  1. Some links belong to orgs / users, not repos, make sure you discard user links and move the org links to the github_organizations if the entire org builds in Scroll
  2. Some repos don't mention Scroll in the code / readme at all, we need a way to make sure the repo uses Scroll

@dghelm
Copy link
Contributor Author

dghelm commented Nov 20, 2023

Awesome, we'll get this fixed soon.

As for showing things are built on/deployed to Scroll, should that reasoning be in the PR? And is there a preference on what that reasoning looks like? Deployment config code, documentation, contract address?

@eherrerosj
Copy link
Contributor

eherrerosj commented Nov 23, 2023

Awesome, we'll get this fixed soon.

Thx, I'll keep an eye out.

As for showing things are built on/deployed to Scroll, should that reasoning be in the PR? And is there a preference on what that reasoning looks like? Deployment config code, documentation, contract address?

There's no need to reason every case. I'd like to understand how you guys are gathering all these repos to validate the approach with you. This will also allow me to adjust our repo classifiers / validators for Scroll if needed, otherwise we will see a too high False Positive rate and will not be able to merge the changes.

@dghelm
Copy link
Contributor Author

dghelm commented Nov 29, 2023

I'd like to understand how you guys are gathering all these repos to validate the approach with you.

So this list has been curated from 2 internal data sources:

A. Repos of projects that have built on us at hackathons (minimum requirement is a smart contract deployment to Scroll Sepolia testnet or Scroll mainnet, only those confirmed to have deployed included)
B. Orgs and repos of projects that have worked with our ecosystem team and are confirmed as deployed on Scroll or Scroll Sepolia (or in the case of infra, support these networks via providing RPC, indexers, etc.)

I think data source B is actually the more confusing one at this point, as we may have included org URLs as repos, and even still, there might not be specific references to Scroll if the project itself is 90% closed source with an open-source website.

My suggestion for moving ahead right now would be to drop source B except for projects where it's extremely clear they're a Scroll-native project, and to seek clarify from your team on where we might "draw the line", especially for multi-chain projects.

@eherrerosj
Copy link
Contributor

My suggestion for moving ahead right now would be to drop source B except for projects where it's extremely clear they're a Scroll-native project, and to seek clarify from your team on where we might "draw the line", especially for multi-chain projects.

Agreed with your suggestion. For fairness to the other projects, let's keep only those repos / orgs where there are clear mentions / signals / code referring to Scroll publicly.

Please keep in mind the data freeze happens on December 14th. Do you think you guys will be ready for a new review round by EOW?

@eherrerosj eherrerosj added the waiting Waiting for requester to reply or do some improvements label Dec 5, 2023
@dghelm
Copy link
Contributor Author

dghelm commented Dec 11, 2023

Sorry for the delay here @eherrerosj -- I think we need to set up better automation and testing on our end before we pass along data from our Partnerships team.

In an effort to get things resolved more quickly, we've cleaned up our data to only include repos from projects that have explicitly deployed to Scroll or Scroll Sepolia. This data should be exposed either in the linked repos or the projects pages for the hackathons that many projects were built during (not included here, but will be available in another public data source we're looking to release soon).

Thanks for being patient with us while we sort this out!

@eherrerosj
Copy link
Contributor

Sorry for the delay here @eherrerosj -- I think we need to set up better automation and testing on our end before we pass along data from our Partnerships team.

In an effort to get things resolved more quickly, we've cleaned up our data to only include repos from projects that have explicitly deployed to Scroll or Scroll Sepolia. This data should be exposed either in the linked repos or the projects pages for the hackathons that many projects were built during (not included here, but will be available in another public data source we're looking to release soon).

Thanks for being patient with us while we sort this out!

sounds good thank you! Could you please solve the issues that the validation step is detecting in the PR?

@dghelm
Copy link
Contributor Author

dghelm commented Dec 12, 2023

As far as I can tell, I just messed up the git merge, and the script shouldn't do this again. Modified to be case-insensitive for sorting, which makes catching failed deduping a bit easier as well.

@dghelm dghelm requested a review from eherrerosj December 13, 2023 17:14
Copy link
Contributor

@eherrerosj eherrerosj left a comment

Choose a reason for hiding this comment

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

could you please check the URLs that are added as repos but in reality are org URLs?

In those cases, please check if the org represents a project building in Scroll:

  • If so, then please create a new ecosystem (new toml file) with the org and link the ecosystem to Scroll using the new eco title in the sub_ecosytems field in Scroll
  • If not, please only keep the repos that mainly use Scroll

url = "https://github.com/orgs/fight-club-dao/repositories"

[[repo]]
url = "https://github.com/orgs/hackashell"
Copy link
Contributor

Choose a reason for hiding this comment

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

org

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops -- didn't know this was a URL pattern. Fixed.

@dghelm dghelm requested a review from eherrerosj December 14, 2023 01:48
@dghelm
Copy link
Contributor Author

dghelm commented Dec 14, 2023

I know we're getting down to the wire here @eherrerosj -- let me know if there's anything else I can do!

@eherrerosj
Copy link
Contributor

eherrerosj commented Dec 17, 2023

I know we're getting down to the wire here @eherrerosj -- let me know if there's anything else I can do!

I've reviewed the PR. Here are some comments.

Invalid repo URLs

There are still some invalid URLs:

https://github.com/MosaicFun/hackathon-permissionless-regenscore/tree/main
https://github.com/ReFi-DeFi-hack-ethprague-2023
https://github.com/beetogether-work
https://github.com/ikigai-labs-xyz/hackathon-submission/tree/4c9d7175f3c7959c8852693279390a7fec0b71a6/smart-contracts
https://github.com/joshocalico/web3getter/tree/test
https://github.com/katxtong/Permissionless-II-Hack/tree/main/Scroll%20Ballot
https://github.com/nfzgg
https://github.com/nicolasmarin/nfnft/tree/main/nfNFT
https://github.com/razacodespython/proofofnurse/tree/main/proofofnurse

You can use my previous suggestion to deal with those.

Repos that might not belong to Scroll

Attached file with a list of 100 repos that might not belong to Scroll. Keep in mind we have a set of heuristics to find Scroll repos and it might not be 100% accurate.

I suggest you take a look at those repos and then remove them from the PR. If you think they belong to Scroll I would appreciate if you can share the places within the code that demonstrate the usage of Scroll. This way I can fix whatever is wrong with out classifiers.

Just so you have an idea of the timeline in our side, we will likely freeze the data at the end of next week so would appreciate if you work on the changes asap, this way we can ensure that your suggested changes are reflected in the 2023 Developer Report.

Thanks!

📎 Scroll - Repos likely not Scroll

@dghelm
Copy link
Contributor Author

dghelm commented Dec 19, 2023

Latest commits standardize URL structure and fix invalid URLs.

We've also implemented an exclude list to avoid the "Repos likely not Scroll" as we only found 2 projects that mention Scroll explicitly in their repo, but for your reference:

  1. zka-fi -- This repo seems to use submodules in an odd way, but following the contracts submodule, you can see the deployment config includes Scroll testnet.
  2. Otherswipe -- They mention their prize which was dependent on a checked deployment, but as it's not implemented in their repo, should be excluded from Dev Report.

Copy link
Contributor

@eherrerosj eherrerosj left a comment

Choose a reason for hiding this comment

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

awesome work, thank you!

@eherrerosj eherrerosj merged commit 2c4b6f2 into electric-capital:master Dec 19, 2023
1 check passed
@dghelm dghelm deleted the update/projects_2023-11-15_18-54 branch December 19, 2023 17:51
platonicsocrates pushed a commit to platonicsocrates/crypto-ecosystems that referenced this pull request Dec 23, 2023
* Update from Ecosystem Data - Developer Report Updater (p_7NCMQz6)

* Update from Ecosystem Data - Developer Report Updater (p_7NCMQz6)

* Update from Ecosystem Data - Developer Report Updater (p_7NCMQz6)

* Update from Ecosystem Data - Developer Report Updater (p_7NCMQz6)

* Update from Ecosystem Data - Developer Report Updater (p_7NCMQz6)

* fix case inisensitivity search
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting for requester to reply or do some improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants