Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.

report marconi target addresses PLT-1009#796

Merged
kayvank merged 19 commits intomainfrom
1009/report-marconi-target-addresses-v2
Nov 9, 2022
Merged

report marconi target addresses PLT-1009#796
kayvank merged 19 commits intomainfrom
1009/report-marconi-target-addresses-v2

Conversation

@kayvank
Copy link
Collaborator

@kayvank kayvank commented Oct 28, 2022

This PR contains:

  • marconi-JSON RPC demo
  • db-utils package, to extract utxo test data from SQLite
  • Serialisation related fixes
  • remove dependencies from plutus-ledger

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reference the ADR in the PR and reference the PR in the ADR (if revelant)
    • Reviewer requested

kayvank and others added 12 commits October 20, 2022 15:49
Update Default ProtocolParameters in Ledger.Params to current values on
mainnet.  This will allow to emulator to use the same ProtocolParameters values as the current mainnet following Vasil HF
Provide a JSON-RPC function to report on user provided address
Add networkId to dbConfig. This is required so that we can convert
addresses to and from cardano to pluts
…input-output-hk/plutus-apps into 1009/report-marconi-target-addresses-v2
add rest endpoint to report on marconi mamba addresses user has
provided.
Add sql utility for examle and demo purpose
Report target addresses, user input addresses, as valid Bech32 addresses
@kayvank kayvank requested a review from koslambrou October 28, 2022 15:28
@kayvank kayvank changed the title 1009/report marconi target addresses v2 eport marconi target addresses PLT-1009 Oct 28, 2022
@kayvank kayvank changed the title eport marconi target addresses PLT-1009 report marconi target addresses PLT-1009 Oct 28, 2022
Add:
- concurrent query for the in-memory utxo indexer.
- eumulator for utxo indexer. This allows for the JSON-RPC-example
server to function without marconi.  However, the example server does
have a dependency on SQLite utxo-db pre-populated as a result of previous
marconi execution.
This will allow the server to run without a need for marconi. In this
case, we are fetching utxo data directly for cold-store, SQLite db
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from 9befcdd to 9b04775 Compare November 3, 2022 16:24
Merge branch 'main' of github.com:input-output-hk/plutus-apps into 1009/report-marconi-target-addresses-v2
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from 295ee3f to b7a429a Compare November 4, 2022 21:49
@kayvank kayvank requested review from berewt and raduom November 4, 2022 21:51
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from b7a429a to 7a4f8d3 Compare November 4, 2022 22:22
@kayvank kayvank requested a review from koslambrou November 4, 2022 22:45
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from 7a4f8d3 to 9b674a6 Compare November 4, 2022 23:08
Changed Cardano.Api From/To Filed SQL mappings to RawByteString
Add test data from preview-testnet
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from 9b674a6 to 23bfc2c Compare November 7, 2022 03:31
Copy link
Contributor

@koslambrou koslambrou left a comment

Choose a reason for hiding this comment

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

LGTM

provide end-point to report user-provided Shelley addresses for
- REST end-point
- CONSOLE
- JSON-RPC
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from 8f7f32e to 267ec3d Compare November 8, 2022 19:09
Copy link
Contributor

@berewt berewt left a comment

Choose a reason for hiding this comment

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

Looks good to me, just a minor comment on the semaphore usage.

Comment on lines +93 to +95
(atomically (takeTMVar qreq ) )
(atomically (takeTMVar qreq ) )
(atomically (putTMVar utxoIndexer ix) )
Copy link
Contributor

@berewt berewt Nov 8, 2022

Choose a reason for hiding this comment

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

The synchronisation here looks expensive: when a query arrives, we wait for it to release a semaphore, to release an ix to release its action before we wait for the end of the transaction.
The first step looks unnecessary to me, but as always concurrency is complex and I may be wrong (if I'm right removing the initialisation in this bracket and the one in the query shouldn't impact the behaviour and save us a bit of time).

Copy link
Collaborator Author

@kayvank kayvank Nov 8, 2022

Choose a reason for hiding this comment

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

Thank you for taking a detailed look at this part, as I could certainly use another pair of eyes here. Although this section is from the toy example demo, its counterpart, indexer.hs is a critical part of the flow, since the potential of locking SQLite is present.
The main things here are:

  • queries take precedence over inserts.
  • SQLite locks when concurrently performing inserts/queries, concurrent inserts or concurrent queries are OK, mixing them is not.

The section of the code you’re referring to is in the toy json-rpc demo and is mocking the Marconi insert operation, so the code resembles that flow from indexer.hs

When the query arrives, we need to stop the inserts and resume them only after they're completed from both cold and hot storage. The initialization section of the bracket, stops Marconi from inserting, and the closing part releases the lock only after queries have been completed.
Changing this section of the code will require changing the indexer.hs section as well. I’ll be happy to discuss that section with you further as it be great to get more performance from that section. However, since that is a more significant change, and not part of your comment, I suggest we proceed with the merge. If we find that indeed the initialization is not required, I'll do a quick patch to address it.

@kayvank kayvank enabled auto-merge (squash) November 8, 2022 23:42
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from e24d2df to dbba01c Compare November 8, 2022 23:56
@kayvank kayvank force-pushed the 1009/report-marconi-target-addresses-v2 branch from dbba01c to fcf25c9 Compare November 9, 2022 00:07
@kayvank kayvank merged commit 33c6e81 into main Nov 9, 2022
@kayvank kayvank deleted the 1009/report-marconi-target-addresses-v2 branch November 9, 2022 00:48
koslambrou pushed a commit that referenced this pull request Apr 6, 2023
* Update Default ProtocolParams to mainnet PLT-1037

Update Default ProtocolParameters in Ledger.Params to current values on
mainnet.  This will allow to emulator to use the same ProtocolParameters values as the current mainnet following Vasil HF

* Report-marconi-target-addresses PLT-1009

Provide a JSON-RPC function to report on user provided address

* report-marconi-target-addresses -PLT-1009

Add networkId to dbConfig. This is required so that we can convert
addresses to and from cardano to pluts

* report marconi-mamba-Target-Addresses PLT-1009

* report marconi-mamba-Target-Addresses PLT-1009

* report-marconi-target-addresses-v2 PLT-1009

Report target addresses, user input addresses, as valid Bech32 addresses

* report-marconi-target-addresses PLT-1009

Add README to db-utils

* report-marconi-target-addresses PLT-1009

Add:
- concurrent query for the in-memory utxo indexer.
- eumulator for utxo indexer. This allows for the JSON-RPC-example
server to function without marconi.  However, the example server does
have a dependency on SQLite utxo-db pre-populated as a result of previous
marconi execution.
This will allow the server to run without a need for marconi. In this
case, we are fetching utxo data directly for cold-store, SQLite db

* report-marconi-target-addresses-v2 PLT-1009

Changed Cardano.Api From/To Filed SQL mappings to RawByteString

* report-marconi-target-addresses -PLT-1009

Add test data from preview-testnet

* ./marconi-mamba/examples/test-json-rpc.http
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.

3 participants