Expose class hashing functionality #2505
Conversation
t00ts
force-pushed
the
t00ts/class-hash-crate
branch
2 times, most recently
from
2a5841b to
e7468b0
Compare
t00ts
force-pushed
the
t00ts/class-hash-crate
branch
from
e7468b0 to
6068ca2
Compare
t00ts
force-pushed
the
t00ts/class-hash-crate
branch
from
6068ca2 to
cdc6d15
Compare
.github/workflows/publish-crates.yml
Outdated
| VERSION=${{ inputs.version }} | ||
| # Update version in each Cargo.toml | ||
| for crate in common crypto serde class-hash; do | ||
| # Update the package version | ||
| sed -i "s/^version = \".*\"/version = \"$VERSION\"/" "crates/$crate/Cargo.toml" | ||
| # Also update any internal dependencies to use the same version | ||
| sed -i "s/{ path = \"..\/common\" }/{ version = \"$VERSION\" }/" "crates/$crate/Cargo.toml" | ||
| sed -i "s/{ path = \"..\/crypto\" }/{ version = \"$VERSION\" }/" "crates/$crate/Cargo.toml" | ||
| sed -i "s/{ path = \"..\/serde\" }/{ version = \"$VERSION\" }/" "crates/$crate/Cargo.toml" | ||
| done |
There was a problem hiding this comment.
I'm not sure this is good practice. In general, this would lead to the published versions to be different compared to what we have in git.
Can't we just verify this instead of changing stuff in Cargo.toml files? Although by the time this runs it's already fairly late, we should just make sure that the release action fails completely in case publishing the crates fails.
There was a problem hiding this comment.
Tracking this in a new issue: #2509
.github/workflows/publish-crates.yml
Outdated
| # First publish crypto as it has no internal dependencies | ||
| publish_with_retry pathfinder-crypto | ||
| sleep 30 | ||
|
|
||
| # Publish common which depends on crypto | ||
| publish_with_retry pathfinder-common | ||
| sleep 30 | ||
|
|
||
| # Publish serde which depends on common and crypto | ||
| publish_with_retry pathfinder-serde | ||
| sleep 30 | ||
|
|
||
| # Finally publish class-hash which depends on common, crypto and serde | ||
| publish_with_retry pathfinder-class-hash |
There was a problem hiding this comment.
Can't we just publish all packages with a single cargo publish command?
There was a problem hiding this comment.
Tracking this in a new issue: #2509
.github/workflows/release.yml
Outdated
|
|
||
| publish-crates: | ||
| name: Publish crates to crates.io | ||
| needs: upload-assets | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Extract version from tag | ||
| id: version | ||
| run: | | ||
| VERSION=${GITHUB_REF#refs/tags/v} | ||
| echo "version=$VERSION" >> $GITHUB_OUTPUT | ||
|
|
||
| - uses: ./.github/workflows/publish-crates.yml | ||
| with: | ||
| version: ${{ steps.version.outputs.version }} |
There was a problem hiding this comment.
Once we publish these crates we can't really update them, right? If so, then this "publish crates" job should be executed only when we publish the actual release on GitHub?
Otherwise it's possible that:
- we tag a new release in git
- this workflow creates the draft release and builds binary assets, then goes on to publish the crates
- the Docker build however fails and we won't be able to fix it properly because these crates can't be re-published
So I this step should only be done after we've actually published the release in a separate workflow triggered by a new release being published:
on:
release:
types: [published]There was a problem hiding this comment.
Tracking this in a new issue: #2509
t00ts
force-pushed
the
t00ts/class-hash-crate
branch
from
cdc6d15 to
8f5f4c2
Compare
This PR exposes the requested class hash functionality in the simplest way possible *
starknet-gateway-typesto a publicly accessible crate.crypto,common,serde) and the new class hash crate itselfclass-hashto crates.io on every pathfinder release.Closes #2466
* Ideally our
commoncrate should be refactored to expose a more organized API/types with cleaner docs. I attempted this a couple times but our macros make it hard to organize types based on their domain/concerns. It's still a WIP, but didn't want to drag this further, so I'll continue some other time.