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

Add BlockSummary type and related changes #3143

Merged
merged 4 commits into from
Mar 14, 2022

Conversation

HeinrichApfelmus
Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus commented Feb 17, 2022

Issue number

ADP-1422, ADP-1425

Overview

Light-mode (Epic ADP-1422) aims to make synchronisation to the blockchain faster by trusting an off-chain source of aggregated blockchain data.

In this pull request, we add the BlockSummary type that represents transaction data contained in a contiguous sequence of blocks. Instead of storing a sequence of Block directly, the BlockSummary gives us a (monadic) query that can be used to obtain transaction data for specific addresses within that sequence.

data BlockSummary m addr txs = BlockSummary
    { from  :: !BlockHeader
    , to    :: !BlockHeader
    , query :: addr -> m txs
    }

The idea is that the query will eventually call out to the off-chain source of aggregated blockchain data. The from and to fields are necessary to preserve the sequential way of how cardano-wallet currently consumes blockchain data.

This pull request mainly adds and rearranges types, and matches lightSync with the MaybeLight type class. However, the implementation of production or consumption of BlockSummary will be tackled in later pull requests.

Details

  • We add a field lightSync to the NetworkLayer. This field will eventually provide the BlockSummary to a ChainFollower. It is set to Nothing for now.
  • We add a type class MaybeLight s which indicates whether a wallet state supports address and transaction discovery with BlockSummary. All instances are currently set to Nothing.
  • We instantiate the BlockSummary type with …
    • addr = Either Address RewardAccount. In other words, we can either get transaction data pertaining to a single Address, or delegation information pertaining to a RewardAccount.
    • … and txs = ChainEvents, which is a sequence of BlockEvents. In turn, the type BlockEvents contains transactions and delegations that appear within a single block; this can be the full block, or a subset of interest.
  • Even in light-mode, we expect to work with sequences of blocks, so the type BlockData tags whether we are consuming a sequence of Block, or a BlockSummary. In the case of a summary, we include information from the MaybeLight class in order to facilitate consumption.

Comments

  • Later on, transaction data will be consumed by adapting applyBlockToUTxO to work with BlockEvents instead of Block.

@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch 5 times, most recently from ed8e9d0 to 24f0c71 Compare February 17, 2022 19:52
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch 2 times, most recently from b7dbd9b to c6d5d50 Compare February 25, 2022 12:33
@HeinrichApfelmus
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Feb 25, 2022
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 25, 2022

try

Build succeeded:

@HeinrichApfelmus HeinrichApfelmus marked this pull request as draft March 3, 2022 12:48
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch 2 times, most recently from 340ba99 to d58cbaf Compare March 4, 2022 16:13
* The `Summary` constructor cannot occur yet, using "error" for this case is still ok.
* Add `MaybeLight s` class which indicates whether an address discovery state support light-mode
* Add `lightSync` field to `NetworkLayer`
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch from d58cbaf to 6901fbe Compare March 4, 2022 17:13
HeinrichApfelmus and others added 2 commits March 4, 2022 19:23
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch from 6901fbe to 6468d57 Compare March 4, 2022 18:23
@HeinrichApfelmus HeinrichApfelmus marked this pull request as ready for review March 4, 2022 21:09
@HeinrichApfelmus
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Mar 4, 2022
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 4, 2022

try

Build failed:

--
-- However, instead of storing the sequence of blocks of directly as a Haskell
-- list, the 'BlockSummary' only provides a 'query' function
-- which looks up all transactions associated to a given addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

in next installements we will extend that and allow discovery of other things, like stake keys, policy keys, certificates, etc. Right?

Copy link
Contributor Author

@HeinrichApfelmus HeinrichApfelmus Mar 14, 2022

Choose a reason for hiding this comment

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

All things that we need in order to do address discovery, indeed.

The type BlockSummary is actually polymorphic in the addr type, and I felt that it would be easier to understand if I kept calling it "address".

In the LightSummary type, we currently instantiate this to addr = Either Address RewardAccount — as this is all the information currently required by applyBlocks.

}

-- | Merge two lists in sorted order. Remove duplicate items.
mergeOn :: Ord key => (a -> key) -> (a -> a -> a) -> [a] -> [a] -> [a]
Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to include some examples here, just to better explain what to expect from mergeOn. I wonder if there is no function like that in Data.List or Data.List.Extra...maybe we can use zipWith inside?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. mergeOn is very similar to a merging function that one would use in a mergesort implementation. I'll add more documentation / an example.

-- NOTE: Currently used the full address,
-- containing both payment and staking parts.
-- We may want to query only for the payment part at some point.
isRelevantTx addr = any ((addr ==) . address) . outputs . snd
Copy link
Contributor

Choose a reason for hiding this comment

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

this (is. ==) will be needed to generalize later as it only survives non-script case. For scripts we will need isShared, here maybe we should sneek isOurs, even at this moment. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the thing is that in light-mode, we cannot use the isOurs functions anymore — the blockchain data source will provide us with a query addr -> m txs (instantiated to e.g. (Either Address RewardAccount) -> m ChainEvents). Then, address discovery will have to put several addr into this query rather than considering full blocks from the chain and filtering them.

The summarizeOnTxOut and filterBlock functions are intended for testing only. The plan is to test light-mode by comparing two paths:

  • list of blocks → filter into query → light-mode address discovery
  • list of blocks → node-mode address discovery

These functions help with the first →.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I see it. in light mode we use query and it is up to caller using BlockSummary how query looks like. So it is upt to caller, ie. consumer of this abstraction, do deal with address management. It is the caller's responsibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. Hence no isOurs here.

Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

very nice abstractions introduced, I like always sorted by slot ChainEvents being monoid, BlockSummary as we discussed. I wonder if isRelevantTx should not be already powered by using isOurs instead of == and prepared to accomodate isShared later. I am waiting for models' applyBlocks impl and first integration test/setup in next PRs of this series

@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1422/BlockSummary branch from 8a764ea to 4068b27 Compare March 14, 2022 11:27
@HeinrichApfelmus
Copy link
Contributor Author

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 14, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit b24c7e8 into master Mar 14, 2022
@iohk-bors iohk-bors bot deleted the HeinrichApfelmus/ADP-1422/BlockSummary branch March 14, 2022 18:27
WilliamKingNoel-Bot pushed a commit that referenced this pull request Mar 14, 2022
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.

2 participants