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

Jump to bottom

Update to tendermint =0.23.2 and remove chrono #1665

Merged
merged 11 commits into from
Dec 16, 2021
Merged

Update to tendermint =0.23.2 and remove chrono #1665

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8d67c15
Bump tendermint-rs versions to 0.23.2
mzabaluev Dec 10, 2021
9b2dec5
modules: Replaced uses of chrono with time-rs
mzabaluev Dec 13, 2021
d2c4a59
relayer: Removed uses of chrono
mzabaluev Dec 13, 2021
fcea486
no-std-check: Remove chrono
mzabaluev Dec 13, 2021
9abacfb
Removed a question comment
mzabaluev Dec 13, 2021
06715dc
Clippy happy
mzabaluev Dec 13, 2021
cb1c8a5
Changelog entries for #1665
mzabaluev Dec 13, 2021
8792d2c
Unpatch Cargo.toml
mzabaluev Dec 13, 2021
bb75e15
Merge branch 'master' into mikhail/tendermint-0.23.2-and-remove-chrono
mzabaluev Dec 15, 2021
7feaaac
Changelog entry on updating to tendermint* =0.23.2
mzabaluev Dec 15, 2021
22d2b3b
Timestamp: streamline conversion from nanoseconds
mzabaluev Dec 16, 2021
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
4 changes: 4 additions & 0 deletions .changelog/unreleased/breaking-changes/ibc/1665-remove-chrono.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Remove `Timestamp` API that depended on the `chrono` crate:
([#1665](https://github.com/informalsystems/ibc-rs/pull/1665)):
- `Timestamp::from_datetime`; use `From<tendermint::Time>`
- `Timestamp::as_datetime`, superseded by `Timestamp::into_datetime`
4 changes: 4 additions & 0 deletions .changelog/unreleased/improvements/ibc/1665-remove-chrono.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- More conventional ad-hoc conversion methods on `Timestamp`
([#1665](https://github.com/informalsystems/ibc-rs/pull/1665)):
- `Timestamp::nanoseconds` replaces `Timestamp::as_nanoseconds`
- `Timestamp::into_datetime` substitutes `Timestamp::as_datetime`
45 changes: 25 additions & 20 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 12 additions & 5 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ exclude = [
]

# [patch.crates-io]
# tendermint = { git = "https://github.com/informalsystems/tendermint-rs", branch = "master" }
# tendermint-rpc = { git = "https://github.com/informalsystems/tendermint-rs", branch = "master" }
# tendermint-proto = { git = "https://github.com/informalsystems/tendermint-rs", branch = "master" }
# tendermint-light-client = { git = "https://github.com/informalsystems/tendermint-rs", branch = "master" }
# tendermint-testgen = { git = "https://github.com/informalsystems/tendermint-rs", branch = "master" }
# tendermint = { git = "https://github.com/informalsystems/tendermint-rs", branch = "v0.23.x" }
# tendermint-rpc = { git = "https://github.com/informalsystems/tendermint-rs", branch = "v0.23.x" }
# tendermint-proto = { git = "https://github.com/informalsystems/tendermint-rs", branch = "v0.23.x" }
# tendermint-light-client = { git = "https://github.com/informalsystems/tendermint-rs", branch = "v0.23.x" }
# tendermint-testgen = { git = "https://github.com/informalsystems/tendermint-rs", branch = "v0.23.x" }

# [patch.crates-io]
# tendermint = { path = "../tendermint-rs/tendermint" }
# tendermint-rpc = { path = "../tendermint-rs/rpc" }
# tendermint-proto = { path = "../tendermint-rs/proto" }
# tendermint-light-client = { path = "../tendermint-rs/light-client" }
# tendermint-testgen = { path = "../tendermint-rs/testgen" }
thanethomson marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion ci/no-std-check/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ sp-std = { version = "3.0.0", default-features = false, optional = true }

# Dependencies that support no_std
bytes = { version = "1.0.1", default-features = false }
chrono = { version = "0.4.19", default-features = false }
contracts = { version = "0.4.0", default-features = false }
crossbeam-channel = { version = "0.5.1", default-features = false }
ed25519 = { version = "1.2.0", default-features = false, features = ["serde"] }
Expand Down
1 change: 0 additions & 1 deletion ci/no-std-check/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use sp_std;
// Supported Imports

use bytes;
use chrono;
use contracts;
use crossbeam_channel;
use ed25519;
Expand Down
14 changes: 7 additions & 7 deletions modules/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mocks = ["tendermint-testgen"]
# Proto definitions for all IBC-related interfaces, e.g., connections or channels.
ibc-proto = { version = "0.13.0", path = "../proto" }
ics23 = { version = "0.6.7", default-features = false }
chrono = { version = "0.4.19", default-features = false }
time = { version = "0.3", default-features = false }
thiserror = { version = "1.0.30", default-features = false }
serde_derive = { version = "1.0.104", default-features = false }
serde = { version = "1.0", default-features = false }
Expand All @@ -42,17 +42,17 @@ sha2 = { version = "0.9.8", default-features = false }
flex-error = { version = "0.4.4", default-features = false }

[dependencies.tendermint]
version = "=0.23.1"
version = "=0.23.2"

[dependencies.tendermint-proto]
version = "=0.23.1"
version = "=0.23.2"

[dependencies.tendermint-light-client]
version = "=0.23.1"
version = "=0.23.2"
default-features = false

[dependencies.tendermint-testgen]
version = "=0.23.1"
version = "=0.23.2"
optional = true

[dev-dependencies]
Expand All @@ -61,8 +61,8 @@ tracing-subscriber = { version = "0.3.3", features = ["fmt", "env-filter", "json
test-log = { version = "0.2.8", features = ["trace"] }
modelator = "0.4.1"
sha2 = { version = "0.9.8" }
tendermint-rpc = { version = "=0.23.1", features = ["http-client", "websocket-client"] }
tendermint-testgen = { version = "=0.23.1" } # Needed for generating (synthetic) light blocks.
tendermint-rpc = { version = "=0.23.2", features = ["http-client", "websocket-client"] }
tendermint-testgen = { version = "=0.23.2" } # Needed for generating (synthetic) light blocks.

[[test]]
name = "mbt"
Expand Down
2 changes: 1 addition & 1 deletion modules/src/applications/ics20_fungible_token_transfer/msgs/transfer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl From<MsgTransfer> for RawMsgTransfer {
sender: domain_msg.sender.to_string(),
receiver: domain_msg.receiver.to_string(),
timeout_height: Some(domain_msg.timeout_height.into()),
timeout_timestamp: domain_msg.timeout_timestamp.as_nanoseconds(),
timeout_timestamp: domain_msg.timeout_timestamp.nanoseconds(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/src/clients/ics07_tendermint/client_def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use std::convert::TryInto;

use ibc_proto::ibc::core::commitment::v1::MerkleProof;
use tendermint::Time;
use tendermint_light_client::components::verifier::{ProdVerifier, Verdict, Verifier};
use tendermint_light_client::types::{TrustedBlockState, UntrustedBlockState};
use time::OffsetDateTime;

use crate::clients::ics07_tendermint::client_state::ClientState;
use crate::clients::ics07_tendermint::consensus_state::ConsensusState;
Expand Down Expand Up @@ -107,7 +107,7 @@ impl ClientDef for TendermintClient {
untrusted_state,
trusted_state,
&options,
Time(chrono::Utc::now()),
OffsetDateTime::now_utc().try_into().unwrap(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not perhaps implement a now_utc() method for the Time struct to avoid directly importing dependencies from the time crate? I'd generally prefer to use interfaces that hide their underlying implementations as far as possible, as it reduces entanglement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The trouble with now_utc is, it requires std. So I assume it can't be part of Tendermint's core API without an std feature gate, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I can look into providing that mechanism. It's probably preferable to avoid mixing newtypes and the types they're supposed to wrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'd still want to provide the TryFrom/Into conversions from/to OffsetDateTime for convenience, and time is not an optional dependency at the moment (though it could be after redoing the internal representation of Time per ADR-010).

);

match verdict {
Expand Down
22 changes: 13 additions & 9 deletions modules/src/clients/ics07_tendermint/consensus_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use crate::prelude::*;

use core::convert::Infallible;

use chrono::{TimeZone, Utc};
use serde::Serialize;
use tendermint::{hash::Algorithm, time::Time, Hash};
use tendermint_proto::google::protobuf as tpb;
use tendermint_proto::Protobuf;

use ibc_proto::ibc::lightclients::tendermint::v1::ConsensusState as RawConsensusState;
Expand Down Expand Up @@ -58,9 +58,15 @@ impl TryFrom<RawConsensusState> for ConsensusState {
type Error = Error;

fn try_from(raw: RawConsensusState) -> Result<Self, Self::Error> {
let proto_timestamp = raw
let prost_types::Timestamp { seconds, nanos } = raw
.timestamp
.ok_or_else(|| Error::invalid_raw_consensus_state("missing timestamp".into()))?;
// FIXME: shunts like this are necessary due to
// https://github.com/informalsystems/tendermint-rs/issues/1053
let proto_timestamp = tpb::Timestamp { seconds, nanos };
let timestamp = proto_timestamp
.try_into()
.map_err(|e| Error::invalid_raw_consensus_state(format!("invalid timestamp: {}", e)))?;

Ok(Self {
root: raw
Expand All @@ -70,9 +76,7 @@ impl TryFrom<RawConsensusState> for ConsensusState {
})?
.hash
.into(),
timestamp: Utc
.timestamp(proto_timestamp.seconds, proto_timestamp.nanos as u32)
.into(),
timestamp,
next_validators_hash: Hash::from_bytes(Algorithm::Sha256, &raw.next_validators_hash)
.map_err(|e| Error::invalid_raw_consensus_state(e.to_string()))?,
})
Expand All @@ -81,10 +85,10 @@ impl TryFrom<RawConsensusState> for ConsensusState {

impl From<ConsensusState> for RawConsensusState {
fn from(value: ConsensusState) -> Self {
let timestamp = prost_types::Timestamp {
seconds: value.timestamp.0.timestamp(),
nanos: value.timestamp.0.timestamp_subsec_nanos() as i32,
};
// FIXME: shunts like this are necessary due to
// https://github.com/informalsystems/tendermint-rs/issues/1053
let tpb::Timestamp { seconds, nanos } = value.timestamp.into();
let timestamp = prost_types::Timestamp { seconds, nanos };

RawConsensusState {
timestamp: Some(timestamp),
Expand Down
3 changes: 1 addition & 2 deletions modules/src/clients/ics07_tendermint/header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use core::cmp::Ordering;

use bytes::Buf;
use chrono::DateTime;
use prost::Message;
use serde_derive::{Deserialize, Serialize};
use tendermint::block::signed_header::SignedHeader;
Expand Down Expand Up @@ -78,7 +77,7 @@ impl crate::core::ics02_client::header::Header for Header {
}

fn timestamp(&self) -> Timestamp {
Timestamp::from_datetime(DateTime::from(self.signed_header.header.time))
self.signed_header.header.time.into()
}

fn wrap_any(self) -> AnyHeader {
Expand Down
7 changes: 1 addition & 6 deletions modules/src/core/ics02_client/client_consensus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use crate::prelude::*;
use core::convert::Infallible;
use core::marker::{Send, Sync};

use chrono::{DateTime, Utc};

use ibc_proto::ibc::core::client::v1::ConsensusStateWithHeight;
use prost_types::Any;
use serde::Serialize;
Expand Down Expand Up @@ -55,10 +53,7 @@ pub enum AnyConsensusState {
impl AnyConsensusState {
pub fn timestamp(&self) -> Timestamp {
match self {
Self::Tendermint(cs_state) => {
let date: DateTime<Utc> = cs_state.timestamp.into();
Timestamp::from_datetime(date)
}
Self::Tendermint(cs_state) => cs_state.timestamp.into(),

#[cfg(any(test, feature = "mocks"))]
Self::Mock(mock_state) => mock_state.timestamp(),
Expand Down
4 changes: 2 additions & 2 deletions modules/src/core/ics04_channel/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ impl TryFrom<Packet> for Vec<Tag> {
key: PKT_TIMEOUT_TIMESTAMP_ATTRIBUTE_KEY.parse().unwrap(),
value: p
.timeout_timestamp
.as_nanoseconds()
.nanoseconds()
.to_string()
.parse()
.unwrap(),
Expand Down Expand Up @@ -960,7 +960,7 @@ mod tests {
destination_channel: "b_test_channel".parse().unwrap(),
data: "test_data".as_bytes().to_vec(),
timeout_height: Height::new(1, 10),
timeout_timestamp: Timestamp::from_datetime(chrono::offset::Utc::now()),
timeout_timestamp: Timestamp::now(),
};
let mut abci_events = vec![];
let send_packet = SendPacket {
Expand Down
Loading