-
Notifications
You must be signed in to change notification settings - Fork 69
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
fix(memo-timeout-time): use u64 instead of string #4047
Conversation
Pull reviewers statsStats of the last 30 days for composable:
|
# run Composable node
nix run "github:ComposableFi/composable/refs/pull/4047/merge" --allow-import-from-derivation --extra-experimental-features "flakes nix-command" --no-sandbox --accept-flake-config --option sandbox relaxed # run local Picasso DevNet (for CosmWasm development)
nix run "github:ComposableFi/composable/refs/pull/4047/merge#devnet-picasso" --allow-import-from-derivation --extra-experimental-features "flakes nix-command" --no-sandbox --accept-flake-config --option sandbox relaxed # CosmWasm on Substrate CLI tool
nix run "github:ComposableFi/composable/refs/pull/4047/merge#ccw" --allow-import-from-derivation --extra-experimental-features "flakes nix-command" --no-sandbox --accept-flake-config --option sandbox relaxed # run cross chain devnet with Dotsama and Cosmos nodes
nix run "github:ComposableFi/composable/refs/pull/4047/merge#devnet-xc-fresh" --allow-import-from-derivation --extra-experimental-features "flakes nix-command" --no-sandbox --accept-flake-config --option sandbox relaxed
# or same with docker
nix build "github:ComposableFi/composable/refs/pull/4047/merge#devnet-xc-image" --allow-import-from-derivation --extra-experimental-features "flakes nix-command" --no-sandbox --accept-flake-config --option sandbox relaxed \
&& docker load --input result && docker run -it --entrypoint bash devnet-xc:latest -c /bin/devnet-xc-fresh |
@@ -15,7 +15,7 @@ pub struct ForwardingMemo { | |||
#[serde(skip_serializing_if = "Option::is_none")] | |||
pub channel: Option<ChannelId>, | |||
#[serde(skip_serializing_if = "Option::is_none")] | |||
pub timeout: Option<String>, | |||
pub timeout: Option<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.
Note that serde uses integer representation for u64. Serde talking to serde will work fine but we may run into problems if this memo would be ever read by JavaScript for example. Perhaps better use Displayed<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.
pub height: Option,
works fine so i just replaced String with u64 for other field.
composable-deps> error[E0658]: use of unstable library feature 'stdsimd' |
typicall error when std dep was added to no std build and cannot compile runtime |
it is very strange: compiles runtime into wasm successfully |
does it mean that i am not able to use u64 in serde??? |
You should be able to use it. It’s more likely that the Cargo.toml updates you’ve made broke the build. Split this PR into two — one updating the dependencies and the other changing type of timeout — to check. |
@@ -13,7 +13,7 @@ impl Map { | |||
receiver: value.receiver, | |||
port: value.port.map(|x| x.to_string()), | |||
channel: value.channel.map(|x| x.to_string()), | |||
timeout: value.timeout, | |||
timeout: value.timeout.and_then(|x| x.parse().ok()), |
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 error should propagate up.
closed because created other pr |
Required for merge:
pr-workflow-check / draft-release-check
is ✅ successMakes review faster:
misc
label if it should not be in release notesReviewers
@
) or used other form of notification of one person who I think can handle best review of this PR