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

Update subxt version #1750

Merged
merged 14 commits into from
Apr 17, 2023
Merged

Update subxt version #1750

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e8a0961
Checkpoint
pmikolajczyk41 Apr 13, 2023
1b7da96
Compiling
pmikolajczyk41 Apr 13, 2023
d4faaa1
blind guess
pmikolajczyk41 Apr 13, 2023
66793ad
blind guess
pmikolajczyk41 Apr 13, 2023
6825517
blind guess
pmikolajczyk41 Apr 13, 2023
a5b4c71
blind guess
pmikolajczyk41 Apr 13, 2023
9615a80
blind guess
pmikolajczyk41 Apr 13, 2023
ba0c1f2
std gating
pmikolajczyk41 Apr 13, 2023
2f9d547
test deps
pmikolajczyk41 Apr 13, 2023
ce7b5b7
import?
pmikolajczyk41 Apr 13, 2023
54d9199
Merge remote-tracking branch 'origin/master' into pmikolajczyk41/bump…
pmikolajczyk41 Apr 14, 2023
3e50b8d
Review
pmikolajczyk41 Apr 14, 2023
4634785
Merge remote-tracking branch 'origin/master' into pmikolajczyk41/bump…
pmikolajczyk41 Apr 17, 2023
a397ca4
Review
pmikolajczyk41 Apr 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions crates/e2e/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ tokio = { version = "1.18.2", features = ["rt-multi-thread"] }
log = { version = "0.4" }
env_logger = { version = "0.10" }
scale = { package = "parity-scale-codec", version = "3.4", default-features = false, features = ["derive"] }
subxt = "0.27.0"
scale-decode = { version = "0.5.0", default-features = false, features = ["derive"] }
scale-encode = { version = "0.1.0", default-features = false, features = ["derive"] }
ascjones marked this conversation as resolved.
Show resolved Hide resolved
subxt = "0.28.0"

# Substrate
pallet-contracts-primitives = "18.0.0"
sp-core = "16.0.0"
sp-keyring = "18.0.0"
sp-runtime = "18.0.0"
sp-weights = "14.0.0"
pallet-contracts-primitives = "23.0.0"
sp-core = "20.0.0"
sp-keyring = "23.0.0"
sp-runtime = "23.0.0"
sp-weights = "19.0.0"

[dev-dependencies]
# Required for the doctest of `MessageBuilder::call`
Expand Down
56 changes: 36 additions & 20 deletions crates/e2e/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use ink_env::Environment;
use ink_primitives::MessageResult;
use pallet_contracts_primitives::ExecReturnValue;
use sp_core::Pair;
#[cfg(feature = "std")]
ascjones marked this conversation as resolved.
Show resolved Hide resolved
use std::{
collections::BTreeMap,
fmt::Debug,
Expand Down Expand Up @@ -279,6 +280,8 @@ where
CallExtrinsic(subxt::error::DispatchError),
/// Error fetching account balance.
Balance(String),
/// Decoding failed.
Decoding(subxt::Error),
}

// We implement a custom `Debug` here, as to avoid requiring the trait
Expand Down Expand Up @@ -307,12 +310,20 @@ where
Error::CallDryRun(_) => f.write_str("CallDryRun"),
Error::CallExtrinsic(_) => f.write_str("CallExtrinsic"),
Error::Balance(msg) => write!(f, "Balance: {msg}"),
Error::Decoding(err) => write!(f, "Decoding: {err}"),
}
}
}

/// A contract was successfully instantiated.
#[derive(Debug, scale::Decode, scale::Encode)]
#[derive(
Debug,
scale::Decode,
scale::Encode,
scale_decode::DecodeAsType,
scale_encode::EncodeAsType,
)]
#[decode_as_type(trait_bounds = "")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need this if trait bounds are empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

because automatically compiler will require bounds for E (in general, for every generic parameter)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps to avoid having this attribute everywhere we could have a blanket impl of DecodeAsType for the Environment trait?

Copy link
Member Author

Choose a reason for hiding this comment

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

we could, but then I would have to provide something reasonable here:

impl DecodeAsType for E {
    fn decode_as_type(input: &mut &[u8], type_id: u32, types: &PortableRegistry) -> Result<Self, Error> {
        todo!()
    }
}

impl EncodeAsType for E {
    fn encode_as_type_to(&self, type_id: u32, types: &PortableRegistry, out: &mut Vec<u8>) -> Result<(), scale_encode::Error> {
        todo!()
    }
}

@jsdw what makes sense here?

Copy link

Choose a reason for hiding this comment

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

Assuming Environment is a type that can't be constructed (eg enum Environment {}) I think some like that would be fine (though I'd panic!("Shouldn't call x on Environment") on the DecodeAsType and maybe unreachable!() on the EncodeAsType one even so).

If it's at all possible to construct an Environment then personally I'd go for the trait bounds everywhere.

(Side note: I'm noting that Encode and Decode don't require such trait bounds; I wonder how that is possible because maybe we can be smarter with EncodeAsType and DecodeAsType in this regard also)

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm... it's a trait and it feels kinda weird to have these panics in the blanket implementation :| I don't have strong opinion; @ascjones (?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a good idea. Just leave as is with the #[decode_as_type(trait_bounds = "")] for now

struct ContractInstantiatedEvent<E: Environment> {
/// Account id of the deployer.
pub deployer: E::AccountId,
Expand All @@ -329,7 +340,14 @@ where
}

/// Code with the specified hash has been stored.
#[derive(Debug, scale::Decode, scale::Encode)]
#[derive(
Debug,
scale::Decode,
scale::Encode,
scale_decode::DecodeAsType,
scale_encode::EncodeAsType,
)]
#[decode_as_type(trait_bounds = "")]
struct CodeStoredEvent<E: Environment> {
/// Hash under which the contract code was stored.
pub code_hash: E::Hash,
Expand Down Expand Up @@ -580,10 +598,9 @@ where
// multiple accounts as part of its constructor!
} else if is_extrinsic_failed_event(&evt) {
let metadata = self.api.client.metadata();
let dispatch_error = subxt::error::DispatchError::decode_from(
evt.field_bytes(),
&metadata,
);
let dispatch_error =
subxt::error::DispatchError::decode_from(evt.field_bytes(), metadata)
.map_err(Error::Decoding)?;
log_error(&format!(
"extrinsic for instantiate failed: {dispatch_error:?}"
));
Expand Down Expand Up @@ -672,10 +689,10 @@ where
break
} else if is_extrinsic_failed_event(&evt) {
let metadata = self.api.client.metadata();
let dispatch_error = subxt::error::DispatchError::decode_from(
evt.field_bytes(),
&metadata,
);
let dispatch_error =
subxt::error::DispatchError::decode_from(evt.field_bytes(), metadata)
.map_err(Error::Decoding)?;

log_error(&format!("extrinsic for upload failed: {dispatch_error:?}"));
return Err(Error::UploadExtrinsic(dispatch_error))
}
Expand Down Expand Up @@ -727,7 +744,7 @@ where
let tx_events = self
.api
.call(
sp_runtime::MultiAddress::Id(message.account_id().clone()),
subxt::utils::MultiAddress::Id(message.account_id().clone()),
value,
dry_run.exec_result.gas_required,
storage_deposit_limit,
Expand All @@ -743,10 +760,9 @@ where

if is_extrinsic_failed_event(&evt) {
let metadata = self.api.client.metadata();
let dispatch_error = subxt::error::DispatchError::decode_from(
evt.field_bytes(),
&metadata,
);
let dispatch_error =
subxt::error::DispatchError::decode_from(evt.field_bytes(), metadata)
.map_err(Error::Decoding)?;
log_error(&format!("extrinsic for call failed: {dispatch_error:?}"));
return Err(Error::CallExtrinsic(dispatch_error))
}
Expand Down Expand Up @@ -788,10 +804,10 @@ where

if is_extrinsic_failed_event(&evt) {
let metadata = self.api.client.metadata();
let dispatch_error = subxt::error::DispatchError::decode_from(
evt.field_bytes(),
&metadata,
);
let dispatch_error =
subxt::error::DispatchError::decode_from(evt.field_bytes(), metadata)
.map_err(Error::Decoding)?;

log_error(&format!("extrinsic for call failed: {dispatch_error:?}"));
return Err(Error::CallExtrinsic(dispatch_error))
}
Expand Down Expand Up @@ -857,7 +873,7 @@ where
.api
.client
.storage()
.at(None)
.at_latest()
.await
.unwrap_or_else(|err| {
panic!("unable to fetch balance: {err:?}");
Expand Down
72 changes: 42 additions & 30 deletions crates/e2e/src/xts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,43 +33,54 @@ use subxt::{
blocks::ExtrinsicEvents,
config::ExtrinsicParams,
rpc_params,
tx,
utils::MultiAddress,
OnlineClient,
};

/// A raw call to `pallet-contracts`'s `instantiate_with_code`.
#[derive(Debug, scale::Encode, scale::Decode)]
pub struct InstantiateWithCode<B> {
#[derive(Debug, scale::Encode, scale::Decode, scale_encode::EncodeAsType)]
#[encode_as_type(trait_bounds = "")]
pub struct InstantiateWithCode<E: Environment> {
#[codec(compact)]
value: B,
gas_limit: Weight,
storage_deposit_limit: Option<B>,
value: E::Balance,
gas_limit: subxt::utils::Static<Weight>,
storage_deposit_limit: Option<E::Balance>,
code: Vec<u8>,
data: Vec<u8>,
salt: Vec<u8>,
}

/// A raw call to `pallet-contracts`'s `call`.
#[derive(Debug, scale::Encode, scale::Decode)]
pub struct Call<E: Environment, B> {
dest: sp_runtime::MultiAddress<E::AccountId, ()>,
#[derive(Debug, scale::Decode, scale::Encode, scale_encode::EncodeAsType)]
#[encode_as_type(trait_bounds = "")]
pub struct Call<E: Environment> {
dest: MultiAddress<E::AccountId, ()>,
#[codec(compact)]
value: B,
gas_limit: Weight,
storage_deposit_limit: Option<B>,
value: E::Balance,
gas_limit: subxt::utils::Static<Weight>,
Copy link

Choose a reason for hiding this comment

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

Weight is a super simple struct, so for a little added robustness/simplicity I wonder whether it's worth just writing your own version that impls EncodeAsType et al

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but sp_weight::Weight comes with pretty broad API and I don't know whether it would be convenient in the end to copy just the struct (copying the whole API seems to be an overkill); I'm leaving the decision to one of this repo maintainers @ascjones ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with a local copy of the type with a minimal API, it is likely to be highly stable.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

storage_deposit_limit: Option<E::Balance>,
data: Vec<u8>,
}

/// A raw call to `pallet-contracts`'s `call`.
#[derive(Debug, scale::Encode, scale::Decode)]
#[derive(Debug, scale::Decode, scale::Encode, scale_encode::EncodeAsType)]
#[encode_as_type(trait_bounds = "")]
pub struct Transfer<E: Environment, C: subxt::Config> {
dest: C::Address,
dest: subxt::utils::Static<C::Address>,
#[codec(compact)]
value: E::Balance,
}

#[derive(
Debug, Clone, Copy, scale::Encode, scale::Decode, PartialEq, Eq, serde::Serialize,
Debug,
Clone,
Copy,
PartialEq,
Eq,
serde::Serialize,
scale::Decode,
scale::Encode,
scale_encode::EncodeAsType,
)]
pub enum Determinism {
/// The execution should be deterministic and hence no indeterministic instructions
Expand All @@ -90,10 +101,11 @@ pub enum Determinism {
}

/// A raw call to `pallet-contracts`'s `upload`.
#[derive(Debug, scale::Encode, scale::Decode)]
pub struct UploadCode<B> {
#[derive(Debug, scale::Encode, scale::Decode, scale_encode::EncodeAsType)]
#[encode_as_type(trait_bounds = "")]
pub struct UploadCode<E: Environment> {
code: Vec<u8>,
storage_deposit_limit: Option<B>,
storage_deposit_limit: Option<E::Balance>,
determinism: Determinism,
}

Expand Down Expand Up @@ -183,11 +195,11 @@ where
dest: C::AccountId,
value: E::Balance,
) -> Result<(), subxt::Error> {
let call = subxt::tx::StaticTxPayload::new(
let call = subxt::tx::Payload::new_static(
"Balances",
"transfer",
Transfer::<E, C> {
dest: dest.into(),
dest: subxt::utils::Static(dest.into()),
value,
},
Default::default(),
Expand Down Expand Up @@ -249,7 +261,7 @@ where
signer: &Signer<C>,
) -> ExtrinsicEvents<C>
where
Call: tx::TxPayload,
Call: subxt::tx::TxPayload,
{
self.client
.tx()
Expand Down Expand Up @@ -292,12 +304,12 @@ where
salt: Vec<u8>,
signer: &Signer<C>,
) -> ExtrinsicEvents<C> {
let call = subxt::tx::StaticTxPayload::new(
let call = subxt::tx::Payload::new_static(
"Contracts",
"instantiate_with_code",
InstantiateWithCode::<E::Balance> {
InstantiateWithCode::<E> {
value,
gas_limit,
gas_limit: subxt::utils::Static(gas_limit),
ascjones marked this conversation as resolved.
Show resolved Hide resolved
storage_deposit_limit,
code,
data,
Expand Down Expand Up @@ -347,10 +359,10 @@ where
code: Vec<u8>,
storage_deposit_limit: Option<E::Balance>,
) -> ExtrinsicEvents<C> {
let call = subxt::tx::StaticTxPayload::new(
let call = subxt::tx::Payload::new_static(
"Contracts",
"upload_code",
UploadCode::<E::Balance> {
UploadCode::<E> {
code,
storage_deposit_limit,
determinism: Determinism::Deterministic,
Expand Down Expand Up @@ -398,20 +410,20 @@ where
/// contains all events that are associated with this transaction.
pub async fn call(
&self,
contract: sp_runtime::MultiAddress<E::AccountId, ()>,
contract: MultiAddress<E::AccountId, ()>,
value: E::Balance,
gas_limit: Weight,
storage_deposit_limit: Option<E::Balance>,
data: Vec<u8>,
signer: &Signer<C>,
) -> ExtrinsicEvents<C> {
let call = subxt::tx::StaticTxPayload::new(
let call = subxt::tx::Payload::new_static(
Copy link

Choose a reason for hiding this comment

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

You could also use Payload::new() to use the documented interface (though downside is it would allocate since it doesn't specialise on static strings)

Copy link
Member Author

Choose a reason for hiding this comment

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

changed to new

"Contracts",
"call",
Call::<E, E::Balance> {
Call::<E> {
dest: contract,
value,
gas_limit,
gas_limit: subxt::utils::Static(gas_limit),
ascjones marked this conversation as resolved.
Show resolved Hide resolved
storage_deposit_limit,
data,
},
Expand Down
4 changes: 4 additions & 0 deletions crates/env/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ secp256k1 = { version = "0.27.0", features = ["recovery", "global-context"], opt
#
# Sadly couldn't be marked as dev-dependency.
# Never use this crate outside the off-chain environment!
scale-decode = { version = "0.5.0", default-features = false, optional = true }
scale-encode = { version = "0.1.0", default-features = false, optional = true }
ascjones marked this conversation as resolved.
Show resolved Hide resolved
scale-info = { version = "2.5", default-features = false, features = ["derive"], optional = true }

[dev-dependencies]
Expand All @@ -63,6 +65,8 @@ std = [
"ink_storage_traits/std",
"ink_engine/std",
"scale/std",
"scale-decode",
"scale-encode",
"scale-info/std",
"secp256k1",
"num-traits/std",
Expand Down
14 changes: 14 additions & 0 deletions crates/env/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ pub trait AccountIdGuard {}
/// used in the [`DefaultEnvironment`].
impl AccountIdGuard for AccountId {}

#[cfg(feature = "std")]
pub trait CodecAsType: scale_decode::DecodeAsType + scale_encode::EncodeAsType {}
#[cfg(feature = "std")]
impl<T: scale_decode::DecodeAsType + scale_encode::EncodeAsType> CodecAsType for T {}
#[cfg(not(feature = "std"))]
pub trait CodecAsType {}
#[cfg(not(feature = "std"))]
impl<T> CodecAsType for T {}
ascjones marked this conversation as resolved.
Show resolved Hide resolved

/// The environmental types usable by contracts defined with ink!.
pub trait Environment {
/// The maximum number of supported event topics provided by the runtime.
Expand All @@ -117,6 +126,7 @@ pub trait Environment {
/// The account id type.
type AccountId: 'static
+ scale::Codec
+ CodecAsType
+ Clone
+ PartialEq
+ Eq
Expand All @@ -127,6 +137,7 @@ pub trait Environment {
/// The type of balances.
type Balance: 'static
+ scale::Codec
+ CodecAsType
+ Copy
+ Clone
+ PartialEq
Expand All @@ -137,6 +148,7 @@ pub trait Environment {
/// The type of hash.
type Hash: 'static
+ scale::Codec
+ CodecAsType
+ Copy
+ Clone
+ Clear
Expand All @@ -149,6 +161,7 @@ pub trait Environment {
/// The type of a timestamp.
type Timestamp: 'static
+ scale::Codec
+ CodecAsType
+ Copy
+ Clone
+ PartialEq
Expand All @@ -159,6 +172,7 @@ pub trait Environment {
/// The type of block number.
type BlockNumber: 'static
+ scale::Codec
+ CodecAsType
+ Copy
+ Clone
+ PartialEq
Expand Down
Loading