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

Change wallet currentTip from SlotId to BlockHeader #384

Merged
merged 8 commits into from
Jun 12, 2019

Conversation

Anviking
Copy link
Collaborator

@Anviking Anviking commented Jun 7, 2019

Issue Number

#219

Overview

  • Change the tip from SlotId to BlockHeader

Comments

  • Remake of First exploration: Jörmungandr NetworkLayer #366
  • The gist of it should be here, but I'll go through and polish / self-review it
  • I have a concern about if we want to extend BlockHeader in the future
  • We could still parameterise over the tip (but even if we solve the Sqlite schema by storing Either tip1 tip2 as (Maybe tip1, Maybe tip2), we introduce a new dimension of complexity in the database, which might be undesired)

@Anviking Anviking self-assigned this Jun 7, 2019
@Anviking
Copy link
Collaborator Author

Anviking commented Jun 7, 2019

This will never work 😕

BlockHeader stores the blockId of the parent. So if the tip is n we must call getDescendants on (n-1). This was fine, I thought — but of course we can't ask for the descendants of block -1 to fetch block 0.

But, I think we can't go wrong with data Cursor = { blockId :: Hash "BlockHeader", header :: BlockHeader }, which also happens to be the exact type used for the current networkTip.

@Anviking Anviking changed the title tip :: BlockHeader Change wallet currentTip from SlotId to BlockHeader Jun 10, 2019
@Anviking Anviking force-pushed the anviking/219/blockheader-tip branch 4 times, most recently from 1890f2a to 59c7c69 Compare June 11, 2019 15:18
@KtorZ KtorZ force-pushed the anviking/219/blockheader-tip branch from 59c7c69 to a746e77 Compare June 11, 2019 18:03
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

👍

@KtorZ KtorZ mentioned this pull request Jun 11, 2019
12 tasks
@KtorZ KtorZ force-pushed the anviking/219/blockheader-tip branch from 715763c to 29a490a Compare June 12, 2019 08:47
@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2019

rebased on top of master + improved comment for defaultRetryPolicy as suggested in #400

@Anviking Anviking force-pushed the anviking/219/blockheader-tip branch 5 times, most recently from df46b6e to dfa4464 Compare June 12, 2019 09:28
@Anviking
Copy link
Collaborator Author

Squashed relevant commits into Change type of tip from SlotId to BlockHeader.

@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2019

@Anviking Not so sure about the squash here. I think some commits deserved to be separated for readability. Styling stuff and indentation fixes, sure; but structural changes were better off separated IMO.

@Anviking Anviking force-pushed the anviking/219/blockheader-tip branch 2 times, most recently from 988ddc1 to 19f89fe Compare June 12, 2019 10:16
@Anviking Anviking force-pushed the anviking/219/blockheader-tip branch from 19f89fe to e46b2fa Compare June 12, 2019 10:21
@Anviking Anviking merged commit 869803f into master Jun 12, 2019
@Anviking Anviking deleted the anviking/219/blockheader-tip branch June 12, 2019 10:42
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.

None yet

2 participants