Conversation
majecty
added
ics
majecty
force-pushed
the
f/ics-building-channel
branch
from
b07b59c to
c6ed486
Compare
The insert function should return the previous value. If the value does not exist, it should return None. However, it is returning rlp([]) instead. Since this is the PoC branch, I fixed this temporarily.
Before this commit, serialization/deserialization does not work together. This commit fix it by moving datagram tag in the inner type and flattening it.
In the datagram code, ORDERED is 1 and UNORDERED is 0. In the ChannelOrder type, ORDERED is 0 and UNORDERED is 1. I made the datagram code to use the ChannelOrder type.
majecty
force-pushed
the
f/ics-building-channel
branch
from
c6ed486 to
8878e76
Compare
| counterparty_channel_identifier: channel_identifier.clone(), | ||
| // Note: the array should be reversed in the future where `connection` becomes an array. | ||
| connection_hops: vec![connection.clone()], | ||
| connection_hops: vec![connection_end.counterparty_connection_identifier], |
There was a problem hiding this comment.
Note that ICS javascript example uses just connection (given as an argument) here. It's ok not to change if you think this is not important.
There was a problem hiding this comment.
Since the word "connection" was used in the channel/manager.rs code for the connection identifier, I used the term "connection_end".
We need a consistent naming rule. What do you prefer? Shall we use connection for the identifier? Shall we use connection for the ConnectionEnd?
There was a problem hiding this comment.
Ah, I was misunderstanding something. The name connection is already used in the function parameter, which represents connectionHops in javascript. And I misunderstood that we use previous here.
About your point, yes you're right but I think we can just stick to our convention (connection for id and connection_end for struct).
| @@ -54,12 +54,17 @@ impl<'a> Manager<'a> { | |||
| } | |||
There was a problem hiding this comment.
What is "Acked-by: Park Juhyung jh@codechain.io" in your commit message? ('Fix connection hop to use the same chain's identifier')
There was a problem hiding this comment.
I added it by mistake. I'll remove it.
There was a problem with the ICS spec. The spec was not distinguishing the connection's identifier in the two chains. Since a connection's identifier is different in each chain, we should use the identifier carefully.
Before this commit, we tried to log errors from executing datagram by checking the return value. The '?' operator, however, returns an error immediately. So I made a wrapper function and check errors in the wrapper function.
Before this commit, we should run the relayer after creating light clients. After this commit, we can run the relayer before creating light clients. It will make us to run tests easier.
majecty
force-pushed
the
f/ics-building-channel
branch
from
8878e76 to
56eb390
Compare
2 participants
I wrote code that builds a channel between two chains.
I found that there is an error while verifying the connection hops. I'll fix it soon after reading the channel spec again.I fixed all the errors.