-
Notifications
You must be signed in to change notification settings - Fork 261
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
Remove Index
type from Config trait
#1074
Remove Index
type from Config trait
#1074
Conversation
subxt/src/config/extrinsic_params.rs
Outdated
@@ -27,7 +27,7 @@ pub trait ExtrinsicParams<Index, Hash>: Debug + 'static { | |||
fn new( | |||
spec_version: u32, | |||
tx_version: u32, | |||
nonce: Index, | |||
nonce: impl Into<u64>, |
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'd keep this simple and just have it take a concrete u64
; it's only called internally anyway, and so let's avoid extra generics unless they make the API more ergonomic for users :)
Looks good to me! I'd just remove the Did you double check that the signed extension itself always wants a u64? That's the one wrinkle that might complicate things a touch! |
Ah, a wrinkle! I had a look at the pub struct CheckNonce<T: Config>(#[codec(compact)] pub T::Index);
impl<T: Config> CheckNonce<T> {
/// utility constructor. Used only in client/factory code.
pub fn from(nonce: T::Index) -> Self {
Self(nonce)
}
} Ie the nonce type is whatever the This means that we would indeed have to use the metadata to find out the size of the Index type in the |
Scrap that; @tadeohepperle pointed out that the nonce type is compact encoded in the signed extension, and so it's irrelevant what numeric type it actually is, so we're still good :) |
* practice TDD * implement a hashmap 2-phases approach * use nicer types * add test for cache filling * adjust test --------- Co-authored-by: James Wilson <[email protected]>
@@ -138,7 +138,7 @@ | |||
//! // here, or can use `create_partial_signed` to fetch the correct nonce. | |||
//! let partial_tx = client.tx().create_partial_signed_with_nonce( | |||
//! &payload, | |||
//! 0, | |||
//! 0u64, |
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.
Nit: no more u64
type needed now :)
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.
Looks great!
fixes #1061
Whereever we previously required a
Config::Index
, now any type that implementsInto<u64>
can be used.