-
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
Fetch metadata discovered on-chain from remote-peers #1776
Conversation
fc7b95d
to
2b6ea47
Compare
385c015
to
d186224
Compare
2b6ea47
to
d0386e5
Compare
d0386e5
to
930a3b3
Compare
930a3b3
to
a52ed79
Compare
91a5dba
to
89e2f23
Compare
traceWith tr $ MsgFetchPoolMetadata pid url | ||
fetchMetadata url hash >>= \case | ||
Left msg -> Nothing <$ do | ||
traceWith tr $ MsgFetchPoolMetadataFailure pid msg |
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.
Wouldn't we want to store the failure in the db to avoid the worker retrying bad links forever?
Or are you handling it? 🤔
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.
Wouldn't we want to store the failure in the db to avoid the worker retrying bad links forever?
For now, failures are just retried every next block. I'll do something a little smarter in another PR. My plan is to keep in-memory some indexes with the failing ones to avoid retrying them more than X every hour. I don't think it's necessary to store anything in the database here, if the wallet is restarted, it'll retry right away and then wait again until the next hour before re-retrying. I see it as a nice feature :)
45f9717
to
2695e52
Compare
1d21cf0
to
586701f
Compare
The workflow is as follows: - Metadata are discovered on chain in pool registration certificates by the stake pool monitoring loop. Every time a new metadata URL and hash are found in a certificate, they are stored in a specific table which acts as a queue. - Then, another worker loop watches consumes items from that queue and fetch metadata corresponding to each item and stores the result in another table for long-term caching. Note: Metadata can't be updated freely since the hash of their content is stored on-chain as part of the registration certificate. New metadata will necessarily means a new certificate will show up, and will go over the whole fetching loop again. I've currently abstracted away the part that is actually fetching the metadata so that, in a first time, we can directly fetch from remote servers directly, whereas in a second time, we can replace this with a deployed centralized aggregation server like SMASH.
In order to define some necessary non-orphan instances for persitent.
…tabase I'll add some property-based testing to make sure this works fine, in particular this is using raw sql to avoid having to do too many queries with too many things stored in-memory.
4aecaaf
to
2a37581
Compare
@@ -613,6 +627,21 @@ data StakePoolMetadata = StakePoolMetadata | |||
-- ^ Absolute URL for the stake pool's homepage link. | |||
} deriving (Eq, Show, Generic) | |||
|
|||
instance FromJSON StakePoolMetadata where | |||
parseJSON = withObject "StakePoolMetadta" $ \obj -> do |
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.
parseJSON = withObject "StakePoolMetadta" $ \obj -> do | |
parseJSON = withObject "StakePoolMetadata" $ \obj -> do |
[ "Read from DB (" <> show (length refs) <> "): " <> show refs | ||
] | ||
assertWith "no more than known entries" | ||
(length refs <= length entries) |
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.
(Nitpick) Superfluous assertion given the next check? Or is this on purpose to fail fast?
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, indeed, I don't think both assertions below can be true and this one false. so it looks like redundant.
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.
Although, perhaps some redundancy is good for friendlier failure reasons 🤷♂️
[ "Read from DB (" <> show (length refs') <> "): " <> show refs' | ||
] | ||
assertWith "fetched metadata isn't listed anymore" | ||
(hash `notElem` ((\(_,_,c) -> c) <$> refs')) |
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.
This seems like an important property, but the names prop_unfetchedPoolMetadataRefs
and propB
makes it difficult to see.
Maybe just merging propA
and propB
would make it clearer.
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.
Maybe just merging propA and propB would make it clearer.
There's nothing really to merge. In theory, we should really have two distinct properties but I was a bit lazy to rewrite the setup and function signature so I just added it quickly after. I could name propA
and propB
a bit better.
-- If there's no metadata, we typically need not to retry sooner than the | ||
-- next block. So waiting for a delay that is roughly the same order of | ||
-- magnitude as the (slot length / active slot coeff) sounds sound. | ||
blockFrequency = ceiling (1/f) * toMicroSecond slotLength |
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.
Don't you want a $
after ceiling
?
λ> f
9.0e-2
λ> sl
10
λ> (1/f) * sl
111.11111111111111
λ> ceiling (1/f) * sl
120
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.
This is an order of magnitude, so it really doesn't matter.
2a37581
to
a58a193
Compare
a58a193
to
c18da60
Compare
bors r+ |
1776: Fetch metadata discovered on-chain from remote-peers r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 854859c 📍 **add logic for fetching metadata from references in the database** The workflow is as follows: - Metadata are discovered on chain in pool registration certificates by the stake pool monitoring loop. Every time a new metadata URL and hash are found in a certificate, they are stored in a specific table which acts as a queue. - Then, another worker loop watches consumes items from that queue and fetch metadata corresponding to each item and stores the result in another table for long-term caching. Note: Metadata can't be updated freely since the hash of their content is stored on-chain as part of the registration certificate. New metadata will necessarily means a new certificate will show up, and will go over the whole fetching loop again. I've currently abstracted away the part that is actually fetching the metadata so that, in a first time, we can directly fetch from remote servers directly, whereas in a second time, we can replace this with a deployed centralized aggregation server like SMASH. - 3ed5c94 📍 **add and serve metadata in integration cluster pool certificates** - 25d3895 📍 **wrap raw 'Text' as 'StakePoolMetadataUrl' newtype** In order to define some necessary non-orphan instances for persitent. - 385c015 📍 **add method for storing metadata and getting next one to fetch from database** I'll add some property-based testing to make sure this works fine, in particular this is using raw sql to avoid having to do too many queries with too many things stored in-memory. # Comments <!-- Additional comments or screenshots to attach if any --> In progress, mostly untested yet. ``` [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.36 UTC] Taking a little break from fetching metadata, back to it in about 0.2s [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.0 ... 0.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.12 ... 2.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Discovered stake pool registration: Registration of c7258ccc owned by [ed25519_pk1va7rf7f7p097545q8nplgwz98n7x70zk0qm8vhfx3q2apedlt7fsrttw4c] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 775af3b2 owned by [ed25519_pk1nl63hryqlfljan9046ztd6ejgfersnzevk6cvy2rlkr2npkmdvcqvezqk6] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 5a7b67c7 owned by [ed25519_pk1vyvapw37ywgugefz0cl9580272w5399m4u4d74mgrwu7st5dd20qsgg22z] [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Fetching metadata for pool c7258ccc from http://localhost:34159/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Successfully fetched metadata for pool c7258ccc: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPA"}, name = "Genesis Pool A", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 775af3b2 from http://localhost:40195/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 775af3b2: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPB"}, name = "Genesis Pool B", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 5a7b67c7 from http://localhost:45173/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 5a7b67c7: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPC"}, name = "Genesis Pool C", description = Just "Lorem Ipsum Dolor Sit Amet.", homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Taking a little break from fetching metadata, back to it in about 0.2s ``` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]> Co-authored-by: IOHK <[email protected]>
Build failed |
bors r+ |
1776: Fetch metadata discovered on-chain from remote-peers r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 854859c 📍 **add logic for fetching metadata from references in the database** The workflow is as follows: - Metadata are discovered on chain in pool registration certificates by the stake pool monitoring loop. Every time a new metadata URL and hash are found in a certificate, they are stored in a specific table which acts as a queue. - Then, another worker loop watches consumes items from that queue and fetch metadata corresponding to each item and stores the result in another table for long-term caching. Note: Metadata can't be updated freely since the hash of their content is stored on-chain as part of the registration certificate. New metadata will necessarily means a new certificate will show up, and will go over the whole fetching loop again. I've currently abstracted away the part that is actually fetching the metadata so that, in a first time, we can directly fetch from remote servers directly, whereas in a second time, we can replace this with a deployed centralized aggregation server like SMASH. - 3ed5c94 📍 **add and serve metadata in integration cluster pool certificates** - 25d3895 📍 **wrap raw 'Text' as 'StakePoolMetadataUrl' newtype** In order to define some necessary non-orphan instances for persitent. - 385c015 📍 **add method for storing metadata and getting next one to fetch from database** I'll add some property-based testing to make sure this works fine, in particular this is using raw sql to avoid having to do too many queries with too many things stored in-memory. # Comments <!-- Additional comments or screenshots to attach if any --> In progress, mostly untested yet. ``` [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.36 UTC] Taking a little break from fetching metadata, back to it in about 0.2s [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.0 ... 0.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.12 ... 2.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Discovered stake pool registration: Registration of c7258ccc owned by [ed25519_pk1va7rf7f7p097545q8nplgwz98n7x70zk0qm8vhfx3q2apedlt7fsrttw4c] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 775af3b2 owned by [ed25519_pk1nl63hryqlfljan9046ztd6ejgfersnzevk6cvy2rlkr2npkmdvcqvezqk6] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 5a7b67c7 owned by [ed25519_pk1vyvapw37ywgugefz0cl9580272w5399m4u4d74mgrwu7st5dd20qsgg22z] [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Fetching metadata for pool c7258ccc from http://localhost:34159/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Successfully fetched metadata for pool c7258ccc: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPA"}, name = "Genesis Pool A", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 775af3b2 from http://localhost:40195/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 775af3b2: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPB"}, name = "Genesis Pool B", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 5a7b67c7 from http://localhost:45173/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 5a7b67c7: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPC"}, name = "Genesis Pool C", description = Just "Lorem Ipsum Dolor Sit Amet.", homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Taking a little break from fetching metadata, back to it in about 0.2s ``` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]> Co-authored-by: IOHK <[email protected]>
Canceled |
bors r+ |
1776: Fetch metadata discovered on-chain from remote-peers r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 854859c 📍 **add logic for fetching metadata from references in the database** The workflow is as follows: - Metadata are discovered on chain in pool registration certificates by the stake pool monitoring loop. Every time a new metadata URL and hash are found in a certificate, they are stored in a specific table which acts as a queue. - Then, another worker loop watches consumes items from that queue and fetch metadata corresponding to each item and stores the result in another table for long-term caching. Note: Metadata can't be updated freely since the hash of their content is stored on-chain as part of the registration certificate. New metadata will necessarily means a new certificate will show up, and will go over the whole fetching loop again. I've currently abstracted away the part that is actually fetching the metadata so that, in a first time, we can directly fetch from remote servers directly, whereas in a second time, we can replace this with a deployed centralized aggregation server like SMASH. - 3ed5c94 📍 **add and serve metadata in integration cluster pool certificates** - 25d3895 📍 **wrap raw 'Text' as 'StakePoolMetadataUrl' newtype** In order to define some necessary non-orphan instances for persitent. - 385c015 📍 **add method for storing metadata and getting next one to fetch from database** I'll add some property-based testing to make sure this works fine, in particular this is using raw sql to avoid having to do too many queries with too many things stored in-memory. # Comments <!-- Additional comments or screenshots to attach if any --> In progress, mostly untested yet. ``` [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.36 UTC] Taking a little break from fetching metadata, back to it in about 0.2s [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.0 ... 0.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Applying blocks [0.12 ... 2.0] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.36 UTC] Discovered stake pool registration: Registration of c7258ccc owned by [ed25519_pk1va7rf7f7p097545q8nplgwz98n7x70zk0qm8vhfx3q2apedlt7fsrttw4c] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 775af3b2 owned by [ed25519_pk1nl63hryqlfljan9046ztd6ejgfersnzevk6cvy2rlkr2npkmdvcqvezqk6] [cardano-wallet.pools-engine:Info:190] [2020-06-18 13:12:14.37 UTC] Discovered stake pool registration: Registration of 5a7b67c7 owned by [ed25519_pk1vyvapw37ywgugefz0cl9580272w5399m4u4d74mgrwu7st5dd20qsgg22z] [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Fetching metadata for pool c7258ccc from http://localhost:34159/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.56 UTC] Successfully fetched metadata for pool c7258ccc: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPA"}, name = "Genesis Pool A", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 775af3b2 from http://localhost:40195/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 775af3b2: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPB"}, name = "Genesis Pool B", description = Nothing, homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Fetching metadata for pool 5a7b67c7 from http://localhost:45173/metadata.json [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Successfully fetched metadata for pool 5a7b67c7: StakePoolMetadata {ticker = StakePoolTicker {unStakePoolTicker = "GPC"}, name = "Genesis Pool C", description = Just "Lorem Ipsum Dolor Sit Amet.", homepage = "https://iohk.io"} [cardano-wallet.pools-engine:Info:192] [2020-06-18 13:12:14.57 UTC] Taking a little break from fetching metadata, back to it in about 0.2s ``` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]> Co-authored-by: IOHK <[email protected]>
Build failed |
☝️
|
Fetching the balance right after making a transaction when the slot length is 0.2s gives actually little guarantee that the transaction doesn't already have been processed!
Sending multiple outputs to a single wallets or to many wallets has literally no differences. Addresses are addresses.
604dc39
to
331d9f2
Compare
bors r+ |
Build succeeded |
Issue Number
Overview
854859c
📍 add logic for fetching metadata from references in the database
The workflow is as follows:
Metadata are discovered on chain in pool registration certificates by
the stake pool monitoring loop. Every time a new metadata URL and hash
are found in a certificate, they are stored in a specific table which
acts as a queue.
Then, another worker loop watches consumes items from that queue and
fetch metadata corresponding to each item and stores the result in
another table for long-term caching.
Note: Metadata can't be updated freely since the hash of their content
is stored on-chain as part of the registration certificate. New
metadata will necessarily means a new certificate will show up, and
will go over the whole fetching loop again.
I've currently abstracted away the part that is actually fetching the
metadata so that, in a first time, we can directly fetch from remote
servers directly, whereas in a second time, we can replace this with a
deployed centralized aggregation server like SMASH.
3ed5c94
📍 add and serve metadata in integration cluster pool certificates
25d3895
📍 wrap raw 'Text' as 'StakePoolMetadataUrl' newtype
In order to define some necessary non-orphan instances for persitent.
385c015
📍 add method for storing metadata and getting next one to fetch from database
I'll add some property-based testing to make sure this works fine,
in particular this is using raw sql to avoid having to do too many queries
with too many things stored in-memory.
Comments
In progress, mostly untested yet.