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.

Sign up for GitHub

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

Jump to bottom

refactor: analyzer/runtime: Store a more-parsed tx in the db #368

Merged
merged 8 commits into from
Mar 30, 2023
Merged

refactor: analyzer/runtime: Store a more-parsed tx in the db #368

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3eff2e9
tests/e2e_regression: Run the API server in run.sh; allows easier ite…
mitjat Mar 27, 2023
9327bb9
api: Return events in deterministic order
mitjat Mar 27, 2023
cd21b56
analyzer/runtime: Store a more-parsed tx in the db
mitjat Mar 25, 2023
20a13af
storage/client: Pull analyzed tx data from db, do not re-analyze on H…
mitjat Mar 27, 2023
25e2bc0
Stop storing raw tx in db
mitjat Mar 28, 2023
c8dbca9
populate and fetch .to, .to_eth, .amount
mitjat Mar 28, 2023
13298d3
lint
mitjat Mar 29, 2023
02ef2d0
Serialize RuntimeTransaction.Fee using default serializer
mitjat Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions api/spec/v1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1250,13 +1250,13 @@ components:
type: object
required: [code]
properties:
module:
type: string
description: The module of a failed transaction.
code:
type: integer
format: uint32
description: The status code of a failed transaction.
module:
type: string
description: The module of a failed transaction.
message:
type: string
description: The message of a failed transaction.
Expand Down Expand Up @@ -1983,7 +1983,7 @@ components:
RuntimeTransaction:
type: object
# NOTE: Not guaranteed to be present: eth_hash, to, amount.
required: [round, index, timestamp, hash, sender_0, nonce_0, fee, gas_limit, gas_used, size, method, body, success]
required: [round, index, timestamp, hash, sender_0, nonce_0, fee, gas_limit, gas_used, size, method, body]
properties:
round:
type: integer
Expand Down Expand Up @@ -2091,7 +2091,9 @@ components:
example: "100000001666393459"
success:
type: boolean
description: Whether this transaction successfully executed.
description: |
Whether this transaction successfully executed.
Can be absent (meaning "unknown") for confidential runtimes.
error:
$ref: '#/components/schemas/TxError'
description: Error details of a failed transaction.
Expand Down
160 changes: 0 additions & 160 deletions api/v1/logic.go
View file
Open in desktop

This file was deleted.

41 changes: 5 additions & 36 deletions api/v1/strict_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,53 +252,22 @@ func (srv *StrictServerImpl) GetRuntimeEvmTokens(ctx context.Context, request ap
}

func (srv *StrictServerImpl) GetRuntimeTransactions(ctx context.Context, request apiTypes.GetRuntimeTransactionsRequestObject) (apiTypes.GetRuntimeTransactionsResponseObject, error) {
storageTransactions, err := srv.dbClient.RuntimeTransactions(ctx, request.Params, nil)
transactions, err := srv.dbClient.RuntimeTransactions(ctx, request.Params, nil)
if err != nil {
return nil, err
}

// Perform additional tx body parsing on the fly; DB stores only partially-parsed txs.
apiTransactions := apiTypes.RuntimeTransactionList{
Transactions: []apiTypes.RuntimeTransaction{},
TotalCount: storageTransactions.TotalCount,
IsTotalCountClipped: storageTransactions.IsTotalCountClipped,
}
for _, storageTransaction := range storageTransactions.Transactions {
apiTransaction, err2 := renderRuntimeTransaction(storageTransaction)
if err2 != nil {
return nil, fmt.Errorf("round %d tx %d: %w", storageTransaction.Round, storageTransaction.Index, err2)
}
apiTransactions.Transactions = append(apiTransactions.Transactions, apiTransaction)
}

return apiTypes.GetRuntimeTransactions200JSONResponse(apiTransactions), nil
return apiTypes.GetRuntimeTransactions200JSONResponse(*transactions), nil
}

func (srv *StrictServerImpl) GetRuntimeTransactionsTxHash(ctx context.Context, request apiTypes.GetRuntimeTransactionsTxHashRequestObject) (apiTypes.GetRuntimeTransactionsTxHashResponseObject, error) {
storageTransactions, err := srv.dbClient.RuntimeTransactions(ctx, apiTypes.GetRuntimeTransactionsParams{}, &request.TxHash)
transactions, err := srv.dbClient.RuntimeTransactions(ctx, apiTypes.GetRuntimeTransactionsParams{}, &request.TxHash)
if err != nil {
return nil, err
}

if len(storageTransactions.Transactions) == 0 {
if len(transactions.Transactions) == 0 {
return apiTypes.GetRuntimeTransactionsTxHash404JSONResponse{}, nil
}

// Perform additional tx body parsing on the fly; DB stores only partially-parsed txs.
apiTransactions := apiTypes.RuntimeTransactionList{
Transactions: []apiTypes.RuntimeTransaction{},
TotalCount: storageTransactions.TotalCount,
IsTotalCountClipped: storageTransactions.IsTotalCountClipped,
}
for _, storageTransaction := range storageTransactions.Transactions {
apiTransaction, err2 := renderRuntimeTransaction(storageTransaction)
if err2 != nil {
return nil, fmt.Errorf("round %d tx %d: %w", storageTransaction.Round, storageTransaction.Index, err2)
}
apiTransactions.Transactions = append(apiTransactions.Transactions, apiTransaction)
}

return apiTypes.GetRuntimeTransactionsTxHash200JSONResponse(apiTransactions), nil
return apiTypes.GetRuntimeTransactionsTxHash200JSONResponse(*transactions), nil
}

func (srv *StrictServerImpl) GetRuntimeEvents(ctx context.Context, request apiTypes.GetRuntimeEventsRequestObject) (apiTypes.GetRuntimeEventsResponseObject, error) {
Expand Down
28 changes: 23 additions & 5 deletions storage/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/dgraph-io/ristretto"
ethCommon "github.com/ethereum/go-ethereum/common"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"

Expand Down Expand Up @@ -1054,21 +1055,38 @@ func (c *StorageClient) RuntimeTransactions(ctx context.Context, p apiTypes.GetR
IsTotalCountClipped: res.isTotalCountClipped,
}
for res.rows.Next() {
var t RuntimeTransaction
t := RuntimeTransaction{
Error: &TxError{},
}
if err := res.rows.Scan(
&t.Round,
&t.Index,
&t.Timestamp,
&t.Hash,
&t.EthHash,
&t.Sender0,
&t.Sender0Eth,
&t.Nonce0,
&t.Fee,
&t.GasLimit,
&t.GasUsed,
&t.Size,
&t.Timestamp,
&t.Raw,
&t.ResultRaw,
&t.AddressPreimage,
&t.Method,
&t.Body,
&t.Success,
&t.Error.Module,
&t.Error.Code,
&t.Error.Message,
); err != nil {
return nil, wrapError(err)
}
if t.Success != nil && *t.Success {
t.Error = nil
}
// Fancy-format eth addresses: Apply checksum capitalization, prepend 0x.
if t.Sender0Eth != nil {
*t.Sender0Eth = ethCommon.HexToAddress(*t.Sender0Eth).Hex()
Copy link
Collaborator

Choose a reason for hiding this comment

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

view logic in storage layer 🙃

}

ts.Transactions = append(ts.Transactions, t)
}
Expand Down
49 changes: 29 additions & 20 deletions storage/client/queries/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,33 +313,42 @@ const (
SELECT
txs.round,
txs.tx_index,
txs.timestamp,
txs.tx_hash,
txs.tx_eth_hash,
signer0.signer_address AS sender0,
encode(signer_eth.address_data, 'hex') AS sender0_eth,
signer0.nonce AS nonce0,
txs.fee,
txs.gas_limit,
txs.gas_used,
txs.size,
txs.timestamp,
txs.raw,
txs.result_raw,
(
SELECT
json_object_agg(pre.address, encode(pre.address_data, 'hex'))
FROM chain.runtime_related_transactions AS rel
JOIN chain.address_preimages AS pre ON rel.account_address = pre.address
WHERE txs.runtime = rel.runtime
AND txs.round = rel.tx_round
AND txs.tx_index = rel.tx_index
) AS eth_addr_lookup
txs.method,
txs.body,
-- .to, .to_eth, and .amout have to be inferred from the body post-hoc
Copy link
Collaborator

Choose a reason for hiding this comment

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

amout -> amount

Copy link
Collaborator

@pro-wh pro-wh Mar 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

what's our strategy for to_eth now? second query? next commit, we decide one during analysis

txs.success,
txs.error_module,
txs.error_code,
txs.error_message
FROM chain.runtime_transactions AS txs
LEFT JOIN chain.runtime_related_transactions AS rel ON txs.round = rel.tx_round
AND txs.tx_index = rel.tx_index
AND txs.runtime = rel.runtime
LEFT JOIN chain.runtime_transaction_signers AS signer0 USING (runtime, round, tx_index)
LEFT JOIN chain.address_preimages AS signer_eth ON
Copy link
Collaborator

Choose a reason for hiding this comment

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

AS signer0_eth

(signer0.signer_address = signer_eth.address) AND
(signer_eth.context_identifier = 'oasis-runtime-sdk/address: secp256k1eth') AND
(signer_eth.context_version = 0)
LEFT JOIN chain.runtime_related_transactions AS rel ON
(txs.round = rel.tx_round) AND
(txs.tx_index = rel.tx_index) AND
(txs.runtime = rel.runtime) AND
-- When related_address ($4) is NULL and hence we do no filtering on it, avoid the join altogether.
-- Otherwise, every tx will be returned as many times as there are related addresses for it.
AND $4::text IS NOT NULL
WHERE (txs.runtime = $1) AND
($2::bigint IS NULL OR txs.round = $2::bigint) AND
($3::text IS NULL OR txs.tx_hash = $3::text OR txs.tx_eth_hash = $3::text) AND
($4::text IS NULL OR rel.account_address = $4::text)
($4::text IS NOT NULL)
WHERE
(txs.runtime = $1) AND
(signer0.signer_index = 0) AND
Copy link
Collaborator

Choose a reason for hiding this comment

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

might be a dumb q, but is it worth moving this line to the join above? eg, LEFT JOIN chain.runtime_transaction_signers AS signer0 USING (runtime, round, tx_index) ON (signer0.signer_index = 0)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not dumb at all, might be a little more readable. I debated it myself. A gotcha is that you cannot mix ON and USING, so we'd have to expand the USING into a much longer ON block, which I think hurts readability overall. Happy to change it if you have stronger feelings here; I don't.

Copy link
Collaborator

Choose a reason for hiding this comment

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

question about this: does factoring out this WHERE signer0.signer_index = 0 change the meaning of the join outer-sidedness? if we tried doing the same for a hypothetical signer1 field, would the dbms conceptually first come up with a record:

method=accounts.Transfer, signer0=oasis1xxxx, signer1=NULL

and then determine NULL != 0 and discard it?

we're lucky here that everything actually has a signer0

Copy link
Collaborator Author

@mitjat mitjat Apr 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought they were equivalent. I now see there's a subtle difference:

a restriction placed in the ON clause is processed before the join, while a restriction placed in the WHERE clause is processed after the join. That does not matter with inner joins, but it matters a lot with outer joins.

See "Notice that placing the restriction in the WHERE clause produces a different result:" here.

Thanks. For our case ... Perhaps we should keep the WHERE even though it's less "pure"? It makes the join less "outer": In the hypothetical case of a tx without a signer, WHERE would discard that tx, whereas ON would return the tx but with a null signer. But our data types don't support null signers. Hrm it's actually good that we'd then crash and return a 500 for a result involving such a tx.

OK let's change the query. It's all pretty philosophical because we always have a signer, but still.

Copy link
Collaborator

Choose a reason for hiding this comment

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

changing in #367

($2::bigint IS NULL OR txs.round = $2::bigint) AND
($3::text IS NULL OR txs.tx_hash = $3::text OR txs.tx_eth_hash = $3::text) AND
($4::text IS NULL OR rel.account_address = $4::text)
ORDER BY txs.round DESC, txs.tx_index DESC
LIMIT $5::bigint
OFFSET $6::bigint
Expand Down
Loading