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

WIP: Initial IBC Spec #608

Merged
merged 13 commits into from
Mar 19, 2018
Merged

WIP: Initial IBC Spec #608

merged 13 commits into from
Mar 19, 2018

Conversation

mappum
Copy link
Contributor

@mappum mappum commented Mar 12, 2018

@adrianbrink, @mossid and I planned out just enough of the IBC spec to start moving forward with implementation, with room to expand to something closer to the design made by @ethanfrey (https://github.com/cosmos/ibc/blob/master/CosmosIBCSpecification.pdf).

closes #602

@mappum mappum requested a review from ebuchman as a code owner March 12, 2018 16:21
@codecov
Copy link

codecov bot commented Mar 12, 2018

Codecov Report

Merging #608 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #608   +/-   ##
========================================
  Coverage    66.09%   66.09%           
========================================
  Files           38       38           
  Lines         2094     2094           
========================================
  Hits          1384     1384           
  Misses         603      603           
  Partials       107      107

@adrianbrink adrianbrink changed the base branch from master to develop March 12, 2018 16:42
@adrianbrink adrianbrink self-requested a review March 12, 2018 16:47
adrianbrink
adrianbrink previously approved these changes Mar 12, 2018
@adrianbrink
Copy link
Contributor

closes #602

}

type IBCTransfer struct {
Destination sdk.Address
Copy link
Member

Choose a reason for hiding this comment

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

dont we need a destination ChainID ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, eventually we will need to add a couple of more fields to IBCTransfer. The goal for the first iteration is to keep everything as simple as possible so that we can get to an end-to-end process the fastest. Once we have an end-to-end process we'll extend everything and build up everything around it.

Copy link
Member

Choose a reason for hiding this comment

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

But what good is an MVP of IBC if we can't send to another chain? Maybe I'm not understanding something ...

Copy link
Contributor

Choose a reason for hiding this comment

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

We will assume that only two zones(hubs) are connected to each other directly, so the destination chain id is determined implicitly. We will add most of security functionalities(destchain checking, lightclient..) on the mvp2.


**Packets**
* Connect to 2 Tendermint RPC endpoints
* Query for IBC outgoing `IBCOutMsg` queue (can poll on a certain time

Choose a reason for hiding this comment

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

If messages have sequence numbers, it can queue for a particular key determined based on next sequence number.

@milosevic
Copy link

milosevic commented Mar 13, 2018

Token transfer between 2 chains using simple IBC API

chain A:

ibc_open_connection(chain_B)

move(50 winks, Alice, deposit_account)
ibc_send(dst_id, ibc_transfer(50 winks, Alice, Bob, 1005 (blocknumber at A))
ibc_register_handler(chain_A, ibc_transfer_ack,
{ tx ->
    if tx.timeout < block_header then {
      burn winks from bonded_account
      ibc_send(dst_id, ibc_transfer_ack(ack)
      }
    else
    {
      move(50 winks, bonded_account, Alice)
      ibc_send(dst_id, ibc_transfer_ack(err))
    }
})
ibc_register_handler(chain_A, ibc_transfer_timeout,
{ tx ->
    move (50 winks, bonded_account, Alice)
    ibc_send(dst_id, ibc_transfer_ack(err))
})

note: ibc_transfer_timeout is always generated after timeout has passed
so ibc does not need to understand the semantics of the application

chain B:

ibc_open_connection(chain_A)
ibc_register_handler(chain_A, ibc_transfer,
{ tx ->
    move(50 winks, bonded_account)
    IBC_send(chain_A, ibc_transfer_ack)
}
ibc_register_handler(chain_A, ibc_transfer_ack,
{ tx ->
    if tx.res == ack then
     move(50 winks, bonded_account, Bob)
    else
      burn winks from bonded account
}

note: we assume chains never totally fail, i.e., they will eventually send the
message back.

@mappum mappum changed the title Initialize IBC Spec WIP: Initial IBC Spec Mar 14, 2018
@jaekwon
Copy link
Contributor

jaekwon commented Mar 14, 2018

Here's the first box transliterated to Golang with minor changes

// app init somewhere
ibcKeeper := ibc.NewKeeper(chain_A, config)
ibcChan := ibcKeeper.NewChannel(chain_B, ...)
ibcTable := ibcKeeper.IBCTable()
ibcTable.RegisterPacketHandler(ibc.PacketTypeAck, func(ctx sdk.Context, pkt ibc.Packet) {
    if pkt.Timeout < block_header then {
      burn winks from bonded_account
      ibcChan.Send(ibc.NewAckPacket(nil, ??))
      }
    else
    {
      move(50 winks, bonded_account, Alice)
      ibcChan.Send(ibc.NewAckPacket(err, nil))
    }
})
ibcTable.RegisterPacketHandler(ibc.PacketTypeTimeout, func(ctx sdk.Context, pkt ibc.Packet) {
    move (50 winks, bonded_account, Alice)
    ibcChan.Send(ibc.NewAckPacket(err, nil))
})

// in DeliverTx somewhere
move(50 winks, Alice, deposit_account)
packet := NewIBCPacket(dst_id, 50, "winks", Alice, Bob, 1005)
ibcChan.Send(packet)

ebuchman
ebuchman previously approved these changes Mar 19, 2018
Copy link
Member

@ebuchman ebuchman left a comment

Choose a reason for hiding this comment

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

Schone!

@ebuchman ebuchman merged commit ca4aae1 into develop Mar 19, 2018
@ebuchman ebuchman deleted the matt/ibc-spec branch March 19, 2018 21:04
@rigelrozanski rigelrozanski mentioned this pull request Mar 27, 2018
@cwgoes
Copy link
Contributor

cwgoes commented Mar 28, 2018

Who sends the ibc_transfer_timeout packet - just the user on chain A trying to claim their escrowed tokens back, in case chain B is unresponsive (never confirms ibc_transfer packet), with a proof that chain B has not confirmed the ibc_transfer packet before the timeout?

Presumably possible for timeout to pass, then ack packet to come later, so I think there needs to be a check for asset de-escrow to ensure Alice can't get her tokens back twice, e.g.

ibc_open_connection(chain_B)

move(50 winks, Alice, deposit_account)
escrowId = hash(50, Alice, nonce)
escrowState[escrowId] = 'escrowed'
ibc_send(dst_id, ibc_transfer(50 winks, Alice, Bob, 1005 (blocknumber at A))
ibc_register_handler(chain_A, ibc_transfer_ack,
{ tx ->
    if tx.timeout < block_header then {
      burn winks from bonded_account
      ibc_send(dst_id, ibc_transfer_ack(ack)
      escrowState[escrowId] = 'finalized'
      }
    else
    {
      if escrowState[escrowId] != 'escrowed' throw
      escrowState[escrowId] = 'reverted'
      move(50 winks, bonded_account, Alice)
      ibc_send(dst_id, ibc_transfer_ack(err))
    }
})
ibc_register_handler(chain_A, ibc_transfer_timeout,
{ tx ->
    if escrowState[escrowId] != 'escrowed' throw
    escrowState[escrowId] = 'reverted'
    move (50 winks, bonded_account, Alice)
    ibc_send(dst_id, ibc_transfer_ack(err))
})

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.

Write IBC MVP Spec
7 participants