-
Notifications
You must be signed in to change notification settings - Fork 44
transfer participants extraction for scan bigquery #1244
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
Conversation
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
I might not have full context here but reward minting is a self-transfer where the only output is your sender change and you are not listed explicitly as a receiver. Aren't you missing that? |
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
It depends on what we mean by "transfer participant". Also, can those be incorporated simply by including the |
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.
Pull Request Overview
This PR introduces an SQL test resource to extract unique transfer participants from Scan BigQuery using JSON extraction. Key changes include adding a new SQL query that declares configuration parameters, extracts JSON arrays from transfer exercises, and filters records based on migration_id and record time.
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
| FROM daml_TransferOutput_jsons | ||
| INNER JOIN UNNEST(daml_TransferOutput_jsons.TransferOutput_array) AS TransferOutput; |
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.
Pulled from the docs here (this is JSON, not a struct, but a similar idea applies)
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.
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
| AND (e.migration_id < migration_id | ||
| OR (e.migration_id = migration_id | ||
| AND e.record_time <= UNIX_MICROS(as_of_record_time))) |
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.
Does this buy us anything given that the request was "all", and we're anyway putting "<= now" ?
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.
A couple things:
- As this is concurrent with ingestion, it's best to choose a time where ingestion has likely stabilized, so that there's no chance of partial data.
- If you refactor or alter the query later, you can compare results knowing it's against the same set of transactions.
Yes just including transfer.sender sounds fine to me. |
…transfer-participants [skip ci] Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
@moritzkiefer-da I'm missing something here. How come you're not listed as the receiver? It's a self-transfer, so you are the receiver, aren't you? Or is it implemented as a "send to nobody", so all outputs are change to the sender? |
send to nobody so it only has the sender change output |
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
- suggested by @moritzkiefer-da; thanks Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
|
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.
Query lgtm, I have two questions:
- What is the plan for testing these queries?
- Have we double checked what the intent is with the people requesting this query? We do have all transfer senders and receivers now. But there are two things I'm unsure about:
2.1. Is the intent really all transfer senders and receivers or is it anyone that ever held CC? Because the latter seems like a much simpler query where you just go over all created events and get the holder. It's almost identical in terms of result. Tap makes a difference but that's not even available on mainnet. Importantly this one is also robust against us adding more choices that create amulet other than AmuletRules_Transfer.
2.2. How do we want to treat lock holders? They are not receivers so if we want to include them we need to do more.
| WITH daml_Transfer_jsons AS ( | ||
| SELECT JSON_VALUE(e.argument, | ||
| -- .transfer.sender | ||
| '$.record.fields[0].value.record.fields[0].value.party') |
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.
is it possible to create functions for these kind of 'paths', or is that too far? probably easy to use for others.
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.
I'm not sure, it depends on what functionality is available in temp function signatures.
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 have a look?
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.
I've changed total-supply to include this (as well as this file); what do you think?
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.
I like it! I think it gives a good basis for reuse and ease of use.
…icipants [skip ci] Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
SQL to retrieve all unique parties that were party to a transfer from ledger beginning up to record time, from Scan BigQuery. We only look at receivers because being a receiver at one point is the only way to become a sender.
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines