-
Notifications
You must be signed in to change notification settings - Fork 217
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 NetworkLayer getNext
for Jörmungandr
#405
Conversation
ErrGetDescendantsNetworkUnreachable e -> | ||
ErrGetBlockNetworkUnreachable e | ||
ErrGetDescendantsParentNotFound e -> | ||
ErrGetBlockNotFound . T.pack . show $ e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... not fond of this raw Text
here. We use Haskell! Let's use type 😶 ..especially when we have a nice Hash "BlockHeader"
we can pass around. Even more context would be nice:
- What's the starting point?
- How many descendant are we trying to fetch?
- What's the parent hash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm… good point. I suppose we manage with Hash "BlockHeader"
. ErrGetBlock
is defined in Cardano.Wallet.Network
, and thus shared, but in practice it will work out since it is not used in http-bridge
.
Do we want to go make ErrGetBlock
completely tailored to Jormungandr
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm. No. These were just a few suggestions. We can at least have the Hash "BlockHeader"
of block. Any other detail that isn't jormungandr-specific is welcome.
35cdb4a
to
ebe0405
Compare
lib/core/src/Cardano/Wallet.hs
Outdated
let tmpTip = BlockHeader | ||
(SlotId maxBound maxBound) | ||
(Hash "<Hash of the block before genesis>") | ||
let tmpCp = initWallet tmpTip s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... what?
We should change initWallet
to not take a block header but the actual first block. This will make the akward call to applyBlocks
superfluous; initializing a wallet does the genesis block application (for what it's worth) and return an initial checkpoint.
3e18f49
to
d3f08d2
Compare
853c723
to
7049c56
Compare
7049c56
to
b606f51
Compare
8d5af8d
to
8975467
Compare
initWallet = Wallet mempty mempty | ||
initWallet block s = | ||
let ((txs, utxo'), s') = prefilterBlock (Proxy @t) block mempty s | ||
in Wallet utxo' (Set.fromList txs) (header block) s' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmm. What about applyBlock block (Wallet mempty mempty (header block) s
?
Feels a bit weird to copy paste partial stuff from applyBlock
, especially because, there could be some transactions that are ours in the genesis block (and there will be actually).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will then transition from
(Wallet mempty mempty (header block) s
to
(Wallet x y (header block) s'
i.e, the header inside the first applyBlock
will be incorrect. I was deliberately trying to avoid this.
I'd be fine with this anyway, if it indeed compiles though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm. It's probably not important for now since there will be no transactions yet. And maybe it's best to leave it for later actually until we have confirmation of how the current mainnet is going to be included in the genesis block. So, leave it be 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be some transactions that are ours in the genesis block
I thought (Set.fromList txs)
would add them though 🤔 ?
{ slotId = SlotId 0 0 | ||
, prevBlockHash = Hash "genesis" | ||
} | ||
, transactions = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
- And Jormungandr
8345fc8
to
1d1e4fa
Compare
Issue Number
#219
Overview
WIP!
Comments
Fetching blocks with
nextBlocks
:stack ghci cardano-wallet-jormungandr
How to do required setup: