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.

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

Local chain implementation & basic tests #422

Merged
merged 22 commits into from
Dec 3, 2020
Merged

Local chain implementation & basic tests #422

merged 22 commits into from
Dec 3, 2020

Conversation

adizere
Copy link
Member

@adizere adizere commented Nov 25, 2020

Closes: #158


For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@codecov-io
Copy link

codecov-io commented Nov 26, 2020

Codecov Report

Merging #422 (3f1c2d7) into master (b1b37f5) will increase coverage by 25.4%.
The diff coverage is 68.9%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #422      +/-   ##
=========================================
+ Coverage    13.6%   39.0%   +25.4%     
=========================================
  Files          69     156      +87     
  Lines        3752   11197    +7445     
  Branches     1374    4308    +2934     
=========================================
+ Hits          513    4377    +3864     
- Misses       2618    6194    +3576     
- Partials      621     626       +5     
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/msgs.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 13.6% <0.0%> (-19.7%) ⬇️
modules/src/ics04_channel/error.rs 75.0% <ø> (+50.0%) ⬆️
modules/src/ics04_channel/msgs/chan_open_ack.rs 65.4% <ø> (ø)
...odules/src/ics04_channel/msgs/chan_open_confirm.rs 63.3% <ø> (ø)
modules/src/ics04_channel/msgs/chan_open_init.rs 69.5% <ø> (ø)
modules/src/ics04_channel/msgs/chan_open_try.rs 68.2% <ø> (ø)
... and 288 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be001eb...c92fb0a. Read the comment docs.

@@ -19,7 +19,7 @@ use crate::ics02_client::error::{Error, Kind};
use crate::ics24_host::identifier::ClientId;
use crate::tx_msg::Msg;

const TYPE_MSG_CREATE_CLIENT: &str = "create_client";
pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgCreateClient";
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: Introduced these public constants in all messages to be able to use them in pattern matching, which is useful in the ICS26 handler:

https://github.com/informalsystems/ibc-rs/blob/a82f53a1b58dc868f3e865d34313bf995e9d7291/modules/src/ics26_routing/handler.rs#L23-L26

Ok(key)
}

fn build_client_state(&self, height: ICSHeight) -> Result<Self::ClientState, Error> {
Copy link
Member Author

Choose a reason for hiding this comment

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

These methods are unchanged, just moved around to appear in the same order here as in trait Chain.

@adizere adizere marked this pull request as ready for review December 2, 2020 13:12
@adizere adizere changed the title Local chain implementation & tests Local chain implementation & basic tests Dec 2, 2020
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

This reverts commit 43bd008.
In anticipation of merging master into this dev branch..
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of comments.

modules/src/ics18_relayer/utils.rs Outdated Show resolved Hide resolved
modules/src/ics26_routing/handler.rs Outdated Show resolved Hide resolved
modules/src/mock/context.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Looks good, thanks Adi!

@adizere adizere merged commit 8ddc688 into master Dec 3, 2020
@adizere adizere deleted the adi/158_local branch December 3, 2020 15:59
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Attemptin integration at the level of Chain trait

* Removed rpc client from trait Chain

* Removed ChainConfig from trait Chain

* Minor fixes as follow-up from informalsystems#364

* Impl generic runtime instantiation.

* Migrated CLIs to use generic runtime spawn.

* Added mock light client. First test (incomplete impl).

* Almost ready to send_tx (mock has to be mutable).

* Added chain executor.

* Mutable chain in send_tx, simplified spawn.After review w/ Anca & Romain

* impl CosmosSDKChain cleanup

* Adapted mock context, ICS18 & 26 to correct abstractions.

* Cleaned up Msg trait, added const TYPE_URL.

* Basic light client impl to complete create client test.

* Removed clippy allow exception

* Updated changelog

* Revert "Updated changelog"

This reverts commit 43bd008.
In anticipation of merging master into this dev branch..

* Redid the changelog

* After Anca's comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local chain type
4 participants