Implement FromStr for Path #1480
Conversation
seanchen1991
requested review from
adizere,
ancazamfir and
romac
as code owners
.changelog/unreleased/improvements/ics24-host-path/1460-path-fromstr.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Beautifully done @seanchen1991! I added some comments. This is already looking great but I'm also trying to use it with basecoin to see if there is anything we can improve.
modules/src/ics24_host/path.rs
Outdated
| Path::Ports(port_id) => write!(f, "ports/{}", port_id), | ||
| Path::Sequences(sequence) => write!(f, "sequences/{}", sequence), | ||
| Path::ChannelEnds(port_id, channel_id) => { | ||
| write!(f, "channelEnds/ports/{}/channels/{}", port_id, channel_id) | ||
| } |
There was a problem hiding this comment.
Question (@ancazamfir @adizere) - Why do some of our paths diverge from the spec, eg. commitments/ports/{identifier}/channels/{identifier}/sequences/{identifier} v/s spec commitments/ports/{identifier}/channels/{identifier}/packets/{sequence}?
There was a problem hiding this comment.
Hmm interesting point, I wasn't aware there's a divergence!
I think the ibc-rs implementation consider as ground truth the ibc-go impl:
which diverges from the spec. It would be worthwhile to highlight this problem in the spec repo.
.changelog/unreleased/improvements/ics24-host-path/1460-path-fromstr.md
Outdated
Show resolved
Hide resolved
…fromstr Merging in upstream changes.
…th-fromstr Pulling in upstream changes.
* Add FromStr<Path> and initial test * Add `parse_client_type` fn and associated test * Add `parse_client_paths` fn * Add `parse_connections` fn * Add more Path parsing logic * Complete Path FromStr logic * Add more Path parsing unit tests * Add changelog entry * Fix changelog entry formatting * Remove stale changelog entry * Move changelog entry under `ibc` component * Move Channels and Sequences to a private enum Co-authored-by: Romain Ruetschi <[email protected]>
Closes: #1460
Description
Implements the
FromStrtrait forPath, along with associated unit tests. This is functionality needed in order to address informalsystems/basecoin-rs#14.For contributor use:
unclog.docs/) and code comments.Files changedin the Github PR explorer.