-
Notifications
You must be signed in to change notification settings - Fork 1
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
Release v0.0.10
#605
Release v0.0.10
#605
Conversation
An exception here is `pallet_treasury`, whose benchmarks are failing right now for some reason.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
||
Finally, users are now able to indicate which hashing algorithm they would like to use during the | ||
signing step. We provide some common ones out of the box, but custom user-provided hashing | ||
algorithms are also supported. | ||
|
||
### Breaking Changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jesse and Peg, I removed a couple things in here because while the did break the API between PRs, they didn't break something that existed in v0.0.9
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
assuming updating the treasuring benchmarks later is not going to cause problems (can't see why it would).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It this metadata re-build relating to #598 or is it something to do with doing the benchmarks here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has to do with the changes to the spec_version
and transaction_version
@@ -1,5 +1,7 @@ | |||
#!/bin/bash | |||
|
|||
set -eux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds some extra checks (e.g unbound variables) and printing (e.g values of variables) when running a Bash script.
In this case I was interested in seeing what packages were being used as part of the benchmark array - and it turned out the array was empty!
The output can be a bit annoying, but since we're not running the script that often I think it's alright. I can also remove this from the committed code and just add it locally when needed if anyone complains lol
@@ -34,7 +34,7 @@ axum ={ version="0.6.18", features=["ws"] } | |||
axum-macros="0.3.7" | |||
|
|||
# Substrate | |||
subxt ={ package="subxt", git="https://github.com/paritytech/subxt.git", tag="v0.32.1" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly related to this PR, but if moving from git dependencies to crates.io dependencies is because of wanting to be able to publish to crates.io ourselves: we have hex = "*"
here and wildcards like that are not allowed on crates.io. (without wanting to get into the discussion about whether we should publish to crates.io)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without wanting to get into the discussion about whether we should publish to crates.io
Actually that's a good point. Most packages right now should be in a publishable state. For the TSS server I think the only other thing we need here is for Jake to publish Programs to crates.
We can totally aim to do that for this release.
Wdyt @jakehemmerle @JesseAbram
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked to Jake, gonna do this in the next release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be missing it but I don't think the TSS version is updated
Right now it's not a problem because we're not really running benchmarks properly anyways. If, for example, we had our reference benchmark machine set up and they failed there - yes that would be a problem.
For now I have not been updating the versions of packages which we intend to publish, which include the TSS server version. Once we start publishing on crates then we can bump those |
I don't love it, gonna approve but my argument here is the tss should be fast forwarded and follow the chain versioning since it would be cleaner and would make a lot more sense especially in the changelog. I also don't think we should be changing our behaviour here for crates. |
I don't totally disagree with you here, but the release cadence of each could be quite different. We could publish updates to the server which don't affect on-chain interactions, but may have breaking changes to the publicly exposed API for example. This is worth a longer discussion for sure. |
One thing to note here - the
pallet_treasury
benchmarks fail to run. I've excluded updating them because I don't want to block the release that, but we should follow up with this.