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

Implement historical headers in ICS03 #4647

Closed
mossid opened this issue Jun 30, 2019 · 9 comments
Closed

Implement historical headers in ICS03 #4647

mossid opened this issue Jun 30, 2019 · 9 comments
Assignees

Comments

@mossid
Copy link
Contributor

mossid commented Jun 30, 2019

In Tendermint, the historical information of the blockchain is stored in Tendermint, not in the application, which means the application cannot access to them while processing the transaction.

The historical block header module can easily implemented, with a BeginBlocker that
pushes ctx.Header() into the KVStore and exposing a getter to other modules. It can take additional argument to limit how many height it will store the headers.

This is needed for ICS 03, which requires to be able to access its past headers.

@zmanian
Copy link
Member

zmanian commented Jun 30, 2019

Isn't the relayer the only party the needs access to this information?

I don't think this needs to be stored in state. The relayer can get this information by querying Tendermint unless i am wrong.

@mossid
Copy link
Contributor Author

mossid commented Jun 30, 2019

To update the client only the relayer need to access this information. However connection handshake requires the chain to inspect its past header in order to check if the other chain has correct header originated from this chain(because the other chain only can have the past header of this chain at the time this chain is inspecting the other chain's state with a known consensus state).

function connOpenTry(
  desiredIdentifier: Identifier, counterpartyConnectionIdentifier: Identifier,
  counterpartyClientIdentifier: Identifier, clientIdentifier: Identifier,
  proofInit: CommitmentProof, proofHeight: uint64,
  timeoutHeight: uint64, nextTimeoutHeight: uint64) {
  assert(getConsensusState().getHeight() <= timeoutHeight)
  counterpartyStateRoot = get(rootKey(connection.clientIdentifier, proofHeight))
  expectedConsensusState = getConsensusState()
  expected = ConnectionEnd{INIT, desiredIdentifier, counterpartyClientIdentifier, clientIdentifier, timeoutHeight}
  assert(verifyMembership(counterpartyStateRoot, proofInit,
                          connectionKey(counterpartyConnectionIdentifier), expected))
  assert(verifyMembership(counterpartyStateRoot, proofInit,
                          consensusStateKey(counterpartyClientIdentifier),
                          expectedConsensusState))
  assert(get(connectionKey(desiredIdentifier)) === null)
  identifier = desiredIdentifier
  state = TRYOPEN
  connection = ConnectionEnd{state, counterpartyConnectionIdentifier, clientIdentifier,
                             counterpartyClientIdentifier, nextTimeoutHeight}
  set(connectionKey(identifier), connection)
}

expectedConsensusState = getConsensusState() is where the chain accessing (one of) is past headers.

@zmanian
Copy link
Member

zmanian commented Jun 30, 2019

That's a little sad. I guess we just need to store a window of past headers.

@mossid
Copy link
Contributor Author

mossid commented Jun 30, 2019

Yes it does not need to be the whole past headers. The blockhash() function in solidity provides access only for past 256 blocks, I think it is a convention we can follow.

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 1, 2019

Makes sense to me, although Ideally we could get these previous headers on the fly directly from Tendermint -- I still don't see why this isn't possible?

@mossid
Copy link
Contributor Author

mossid commented Jul 18, 2019

I think it will include async communication between the app layer and tendermint layer on deliverTx execution, possibly causing some nondeterminism? I couldn't think an example case that this will cause a problem now but it seems safe to keep the deliverTx a single round operation. Tendermint core team will know about this better @alexanderbez

@fedekunze fedekunze added this to the IBC Implementation milestone Dec 10, 2019
@fedekunze fedekunze changed the title Implement historical block headers module Implement historical headers to ICS03 Dec 10, 2019
@fedekunze fedekunze changed the title Implement historical headers to ICS03 Implement historical headers in ICS03 Dec 10, 2019
@AdityaSripal AdityaSripal self-assigned this Dec 12, 2019
@fedekunze
Copy link
Collaborator

@AdityaSripal now that ADR17 was merged, we can incorporate those changes to the ICS03 Connection keeper.

There are a few comments that say // XXX: blocked by #5078 on x/ibc/03-connection/keeper/handshake.go which require the historical info introspection.

@fedekunze fedekunze assigned fedekunze and unassigned AdityaSripal Jan 2, 2020
@fedekunze
Copy link
Collaborator

I'll start working on this tomorrow

@fedekunze
Copy link
Collaborator

closed via #5475

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

No branches or pull requests

5 participants