Skip to content
This repository was archived by the owner on Aug 18, 2020. It is now read-only.

Conversation

avieth
Copy link
Contributor

@avieth avieth commented Aug 19, 2019

Previously it would use only one checkpoint: the current tip. That's fine if there are no forks, but wouldn't work if there were.

Now the checkpoints are computed in the same way as the chain sync client from ouroboros-consensus: fibonacci numbers including 0 and k without the duplicate 1.

I've also included some unrelated improvements to documentation, and a commit to make configuring the Byron-side easier (using cardano-sl configurations).

@avieth avieth requested review from coot and dcoutts August 19, 2019 18:54
@avieth avieth force-pushed the avieth/chain_download_checkpoints branch from 7083cb6 to 9c4e8e0 Compare August 29, 2019 23:49
-- is not constant over a whole blockchain: it can be changed by an update.
-- However, in cardano-byron-proxy it is only used to determine message
-- size limits on Byron protocols, so as long as header, block, and tx
-- size limits do not change, it's ok.
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to add this to top level description of byron-proxy, e.g. README.md or a wiki page, if devops should be awere of this.

Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

the cardano-byron-proxy only needs to relay transactions. All other
atomic (non-block) data can be ignored.
Also corrected the download loop to work even if the peer does not
support streaming: the hash of the last block downloaded is retained and
used at the next loop.
Somebody decided to stylish-haskell the imports and it was a mess
@avieth avieth force-pushed the avieth/chain_download_checkpoints branch from 9c4e8e0 to e17fa24 Compare September 2, 2019 20:34
@avieth
Copy link
Contributor Author

avieth commented Sep 2, 2019

bors r+

iohk-bors bot added a commit that referenced this pull request Sep 2, 2019
27: Byron chain download uses better checkpoints r=avieth a=avieth

Previously it would use only one checkpoint: the current tip. That's fine if there are no forks, but wouldn't work if there were.

Now the checkpoints are computed in the same way as the chain sync client from ouroboros-consensus: fibonacci numbers including `0` and `k` without the duplicate `1`.

I've also included some unrelated improvements to documentation, and a commit to make configuring the Byron-side easier (using cardano-sl configurations).

Co-authored-by: Alexander Vieth <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 2, 2019

@iohk-bors iohk-bors bot merged commit e17fa24 into master Sep 2, 2019
@iohk-bors iohk-bors bot deleted the avieth/chain_download_checkpoints branch September 2, 2019 20:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants