Emerald: import parsing from nexus #141

pro-wh · 2022-09-26T19:55:33Z

import from https://github.com/oasisprotocol/oasis-nexus/pull/1

archival note: backed up at #644

pro-wh · 2022-09-27T17:55:47Z

big, chunky todos

improve testing to be as easy as it was for early days oasis-nexus
file away items from analyzer/uncategorized into appropriate locations
file away visitor handler logic into analyzer/modules
unite block_extra and transaction_extra with existing tables
spread small things like CloseOrLog around to rest of repo

mitjat · 2022-09-30T21:45:41Z

Nice!

big, chunky todos

Do you intend to address these in a follow-up PR or in this one? I'm in favor of the former, but either way. Just so long as the scope of reviewing is clear.

pro-wh · 2022-09-30T21:51:45Z

yeah I'll work on them in this pr

…

On Fri, Sep 30, 2022, 2:45 PM mitjat ***@***.***> wrote: Nice! big, chunky todos Do you intend to address these in a follow-up PR or in this one? I'm in favor of the former, but either way. Just so long as the scope of reviewing is clear. — Reply to this email directly, view it on GitHub <#141 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJDVILUVHID2ESSFYSC4SFDWA5NRBANCNFSM6AAAAAAQWD3IBM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Disclaimer: This e-mail and any attachments may contain confidential information. If you are not the intended recipient, any disclosure, copying, distribution or use of any information contained herein is strictly prohibited. If you have received this transmission in error, please immediately notify the sender and destroy the original transmission and any attachments without reading or saving.

pro-wh · 2022-09-30T23:58:23Z

clerical note: moved to https://docs.google.com/document/d/1moqmGcHosdnVUHfYT6M4mpQbWsWEnmHIOpFAfcosZdY/edit#heading=h.imth7ku5x6is

here's my plan for integrating nexus db table/indexes:

db unification

oasis_3.emerald_rounds
- block_extra.num_transactions will go into extra_data
- block_extra.gas_used will go into extra_data
  - this will somewhat duplicate oasis_3.emerald_gas_used
- block_extra.size will go into extra_data
- unordered index added on block_hash
oasis_3.emerald_transactions
- new table
  - indexer currently has separate tables for different transaction-related events
  - I believe we had always desired to have a table of transactions as well, just it had not been implemented
  - based on transaction_extra from nexus, although in indexer style, there is no chain_alias column, and Emerald gets its own table
- height numeric
  - height is the round, but indexer calls it height
  - nexus used bigint for round. is it more expensive to index numeric columns?
- tx_index integer
- primary key (height, tx_index)
- tx_hash text
  - nexus used char(64) for hashes. is it more expensive to index text columns for lookup by equality?
- eth_hash text null
- is there desire for an extra_data json column?
- is there desire for a bytea of the actual transaction?
  - I see consensus oasis_3.transaction has body bytea, was this really only the body?
  - note that to
- unordered index on tx_hash
- unordered index on eth_hash
- no connection with existing emerald_transfers, ... etc event-specific tables 🤔
oasis_3.emerald_transaction_signer
- new table
  - justification: some interactions, notably deposit+withdraw results, need to be looked up by signer and nonce
  - based on transaction_signer from nexus
- height numeric
- tx_index integer
- signer_index integer
- primary key (height, tx_index, signer_index)
- addr text
- nonce integer
- index on (addr, nonce)
oasis_3.emerald_related_transaction
- new table
  - justification: for showing a list of related transactions for a given account
  - based on related_transaction from nexus
  - may alternatively be possible by querying many event tables and merging results?
  - this approach preserves transaction order though
  - this approach leaves out non-transaction events
- account_address text
- tx_height numeric
- tx_index integer
- index on (account_address, tx_height, tx_index)
oasis_1.address_preimage
- new table
  - justification: determine ethereum address corresponding to oasis address, explorer UI will probably use only ethereum addresses where possible
  - justification: determine public key corresponding to oasis address, for registry lookups
  - namespace is oasis_1, referring to the earliest hardfork using compatible address derivation schemes
  - don't forsee hardforks ever to change the derivation scheme; shouldn't need to move to a new table unless it does
  - analysis of older hardforks can benefit from information obtained while analyzing newer hardforks
  - data collected is not isolated to Emerald runtime
- address text primary key
- context_identifier text
- context_version integer
- addr_data bytea

pro-wh · 2022-10-01T00:00:16Z

oh I read that wrong, you prefer to address these in separate PRs? that may be fine, as long as we have an understanding that we'll be editing the 0011 migration instead of doing further migrations on top. it's just too rough right now imo

mitjat · 2022-10-01T01:29:55Z

Oh that's a good writeup on table migrations 👌 . I copy-pasted it into https://docs.google.com/document/d/1moqmGcHosdnVUHfYT6M4mpQbWsWEnmHIOpFAfcosZdY/edit#heading=h.imth7ku5x6is (because github is not great for commenting on comments piecemeal), and added comments. Not 100% done yet but dinner calls; will wrap up probably Monday.

If you prefer to make all (or some of) the changes in this PR, that's fine with me also. I was thinking that submitting early is more for your benefit -- PRs that drag out forever and need to be rebased a thousand times are no fun. They're also no fun for reviewing; though in this particular case, probably the easiest thing for reviewing would be to first get in a baseline PR that changes the indexer where it should conform to nexus (I left some suggestions to that effect, mostly around data types; see if you agree), then submitting this PR neatly cleaned up. But I think you should prioritize your time/sanity with this one. It's small enough that it won't be too huge a pain to review no matter what.

Either way, don't worry about migrations. There are no clients for the indexer yet, so it's trivial to take it offline in prod and have it reindex everything. Just check let the indexer team know before you nuke the db so we all expect the outage. Also, I can help with all ops-related stuff like this.

pro-wh · 2022-10-03T19:39:00Z

ok let's do this in separate PRs

pro-wh · 2022-10-03T23:31:19Z

storage/migrations/0011_emerald_incoming.up.sql

+BEGIN;
+
+CREATE TABLE block_extra (
+                             chain_alias VARCHAR(32),


what is this indentation

Weirdness indeed.

Assuming you used an autoformatter and you're not happy with it: I looked around and tested a few; sqlfmt by Cockroach Labs looks best. See writeup, demo (increase max width with non-obvious GUI slider), binary.

~~I don't feel motivated enough to integrate it into our CI, but it's where we should head eventually, probably.~~
I sunk a good chunk of time into this before realizing that contrary to their docs, they can ingest only some postgres-style SQL, and they cannot emit it. For example, TEXT is consistently changed to STRING.

shouldn't have to worry about this in this PR, gonna address in #147

aefhm · 2022-10-04T04:04:34Z

table migrations

We did agree (verbally I believe) to squash current migrations.

pro-wh · 2022-10-07T23:22:10Z

reviewers, can we merge this?

mitjat

Apologies for letting this one sit unattended; I thought it was blocked on #147, not the other way around.

mitjat · 2022-10-08T01:56:03Z

analyzer/emerald/emerald.go

-	// TODO(ennsharma): Process Emerald transaction data in accordance with
-	// https://github.com/oasisprotocol/emerald-web3-gateway/blob/main/indexer/utils.go#L225
+	var rtid ocCommon.Namespace
+	if err := rtid.UnmarshalHex("000000000000000000000000000000000000000000000000e2eaa99fc008f87f"); err != nil {


Nit: Extract to top-level constant

doing this in #158

mitjat · 2022-10-08T01:58:19Z

analyzer/emerald/emerald.go

+	if err := rtid.UnmarshalHex("000000000000000000000000000000000000000000000000e2eaa99fc008f87f"); err != nil {
+		return err
+	}
+	sigContext := signature.DeriveChainContext(rtid, "b11b369e0da5bb230b220127f5e7b242d385ef8c6f54906243f30af63c815535")


Can we fetch this from the node instead of hardcoding? This same emerald parser will run for emerald v1 (or whatever the current version is) on oasis-4.

unhardcoded in #158

analyzer/emerald/incoming.go

mitjat · 2022-10-08T05:38:35Z

analyzer/emerald/incoming.go

+		tx, err := common.VerifyUtx(sigContext, &txr.Tx)
+		if err != nil {
+			err = fmt.Errorf("tx %d: %w", i, err)
+			fmt.Println(err)


We should pull in and use the logger, like other modules do. I'm fine to do it in a follow-up PR, though in that case let's create a doc/issue/whatever where we can track those "to do later" tasks.

let's do this in #158

mitjat · 2022-10-08T05:41:35Z

analyzer/emerald/incoming.go

+	blockData.NumTransactions = len(txrs)
+	blockData.TransactionData = make([]*BlockTransactionData, 0, len(txrs))
+	blockData.AddressPreimages = map[string]*AddressPreimageData{}
+	for i, txr := range txrs {


Nit: i is good for a short-lived local variable, but this one lives over ~200 lines. Let's call it tx_idx or something that still makes sense (and is not in danger of being shadowed) 100 lines lower.

let's do this in #158

analyzer/emerald/incoming.go

mitjat · 2022-10-08T06:13:13Z

analyzer/uncategorized/evm_raw_tx_test.go

+func decodeExpectCall(
+	t *testing.T,
+	raw string,
+	expectedChainID uint64,


The high number of args makes tests hard to read. Can we introduce a type Call struct {...}, then having expected Call here and Call { ChainId: ..., To:, ..., ... } in tests?

Same for decodeExpectCreate.

ya that'd be good. let's do that in #158

pro-wh · 2022-10-10T20:45:34Z

let's make most changes in #158 or later. that PR will be moving things around, so it would be more work to adjust things here and rebase #158

From d14dc39

Co-authored-by: mitjat <[email protected]>

mitjat

Thank you for responding to all the comments! I left the threads unresolved for easier tracking, but I see you've nicely documented them in the description of #158 already, so do with the threads here as you please :)

analyzer/emerald/incoming.go

pro-wh · 2022-10-11T18:25:18Z

Error: cannot use batch (variable of type *storage.QueryBatch) as *pgx.Batch value in argument to emitRoundBatch (typecheck)

ugh what's going on in main

pro-wh · 2022-10-11T18:30:39Z

#154 changed the batch type

pro-wh added the analysis-layer Analysis layer-related issues. label Sep 26, 2022

pro-wh requested review from ennsharma, aefhm and Andrew7234 as code owners September 26, 2022 19:55

pro-wh force-pushed the pro-wh/feature/emerald branch from 5b7321d to 06ef29e Compare September 26, 2022 22:31

pro-wh force-pushed the pro-wh/feature/emerald branch from 06ef29e to a4e7754 Compare October 3, 2022 19:42

pro-wh changed the title ~~emerald work area~~ emerald tx parsing import Oct 3, 2022

pro-wh commented Oct 3, 2022

View reviewed changes

pro-wh force-pushed the pro-wh/feature/emerald branch from a4e7754 to 6461517 Compare October 7, 2022 23:21

pro-wh changed the title ~~emerald tx parsing import~~ Emerald: import parsing from nexus Oct 7, 2022

pro-wh force-pushed the pro-wh/feature/emerald branch from 6461517 to 44dcdae Compare October 7, 2022 23:26

mitjat reviewed Oct 8, 2022

View reviewed changes

pro-wh mentioned this pull request Oct 10, 2022

Emerald: unhardcode runtime ID and chain context #158

Merged

pro-wh and others added 5 commits October 10, 2022 15:58

Emerald: 0.01 effort paste from nexus

b8dfe75

From d14dc39

go mod tidy

3207232

Migration for our schema

b70e97f

Separated lint

4f40268

Emerald: add comment heading

540f441

Co-authored-by: mitjat <[email protected]>

pro-wh force-pushed the pro-wh/feature/emerald branch from bb2ee02 to 540f441 Compare October 10, 2022 22:59

mitjat approved these changes Oct 11, 2022

View reviewed changes

analyzer/emerald/incoming.go Show resolved Hide resolved

pro-wh force-pushed the pro-wh/feature/emerald branch from 540f441 to e1b4d3a Compare October 11, 2022 18:20

pro-wh added 2 commits October 11, 2022 11:27

Merge branch 'main' into pro-wh/feature/emerald

5c005b7

Emerald: update to storage.QueryBatch

209444c

pro-wh force-pushed the pro-wh/feature/emerald branch from e1b4d3a to 209444c Compare October 11, 2022 18:28

pro-wh merged commit 33948ea into main Oct 11, 2022

pro-wh deleted the pro-wh/feature/emerald branch October 11, 2022 18:32

pro-wh mentioned this pull request Oct 11, 2022

Emerald: nexus incoming code Go changes #166

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emerald: import parsing from nexus #141

Emerald: import parsing from nexus #141

pro-wh commented Sep 26, 2022 •

edited

Loading

pro-wh commented Sep 27, 2022

mitjat commented Sep 30, 2022

pro-wh commented Sep 30, 2022 via email

pro-wh commented Sep 30, 2022 •

edited

Loading

pro-wh commented Oct 1, 2022

mitjat commented Oct 1, 2022 •

edited

Loading

pro-wh commented Oct 3, 2022

pro-wh Oct 3, 2022

mitjat Oct 5, 2022 •

edited

Loading

pro-wh Oct 5, 2022

aefhm commented Oct 4, 2022

pro-wh commented Oct 7, 2022

mitjat left a comment

mitjat Oct 8, 2022

pro-wh Oct 10, 2022

mitjat Oct 8, 2022

pro-wh Oct 10, 2022

mitjat Oct 8, 2022

pro-wh Oct 10, 2022

mitjat Oct 8, 2022

pro-wh Oct 10, 2022

mitjat Oct 8, 2022

pro-wh Oct 10, 2022

pro-wh commented Oct 10, 2022 •

edited

Loading

mitjat left a comment

pro-wh commented Oct 11, 2022

pro-wh commented Oct 11, 2022

Emerald: import parsing from nexus #141

Emerald: import parsing from nexus #141

Conversation

pro-wh commented Sep 26, 2022 • edited Loading

pro-wh commented Sep 27, 2022

mitjat commented Sep 30, 2022

pro-wh commented Sep 30, 2022 via email

pro-wh commented Sep 30, 2022 • edited Loading

pro-wh commented Oct 1, 2022

mitjat commented Oct 1, 2022 • edited Loading

pro-wh commented Oct 3, 2022

Choose a reason for hiding this comment

mitjat Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aefhm commented Oct 4, 2022

pro-wh commented Oct 7, 2022

mitjat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh commented Oct 10, 2022 • edited Loading

mitjat left a comment

Choose a reason for hiding this comment

pro-wh commented Oct 11, 2022

pro-wh commented Oct 11, 2022

pro-wh commented Sep 26, 2022 •

edited

Loading

pro-wh commented Sep 30, 2022 •

edited

Loading

mitjat commented Oct 1, 2022 •

edited

Loading

mitjat Oct 5, 2022 •

edited

Loading

pro-wh commented Oct 10, 2022 •

edited

Loading