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.

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

Index pending transactions #255

Merged
merged 11 commits into from
Jun 7, 2018
Merged

Index pending transactions #255

merged 11 commits into from
Jun 7, 2018

Conversation

KronicDeth
Copy link
Contributor

@KronicDeth KronicDeth commented Jun 6, 2018

Resolves #250

Changelog

Enhancements

  • Document AddressExtraction.extract_addresses.
  • Use EthereumJSONRPC.request/1 in EthereumJSONRPC.Parity.
  • Rename Indexer.BlockFetcher.monitor_task to Indexer.start_monitor and make it public, so that all *Fetchers can use it.
  • Check for required timestamps option as soon as possible as not including timestamps is a developer error, so it should be caught as soon as possible to allow the code to be fixed in development.
  • Use parity_pendingTransactions JSONRPC to get the pending transactions and import them. If they conflict with pre-existing transactions, then the previous transactions win, under the assumption that sometimes pending transactions repository transaction will COMMIT after the the
    realtime index has COMMITed that same transaction being validated as the timeout for transactions is 60 seconds while the block rate is faster than that.

Bug Fixes

  • Move AddressExtraction out of BlockFetcher because it is used in the
    InternalTransactionFetcher and PendingTransactionFetcher.
  • Pending transactions have nil for "blockNumber" and "transactionIndex", which broke quantity_to_integer, so for those keys, check if nil and pass through. For others, check that the value is not nil as it gives a better error message with both the key and value instead of just the nil value passed to quantity_to_integer as happened before.
  • Exclude pending transactions for unfetched internal transactions as Parity doesn't support asking for the internal transactions of pending transactions.
  • Add :transactions :on_conflict option that is required when :transactions :params as passed because the repository transaction for a pending Explorer.Chain.Transactions could COMMIT after the repository transaction for that same transaction being collated into a block, writers, it is recommended to use :nothing for pending transactions and :replace_all for collated transactions, so that collated transactions win.
  • params_option value should be [map], not map since it is a list of params for the given schema.
  • Fix typo adddresses -> addresses in addresses_option.

@coveralls
Copy link

coveralls commented Jun 6, 2018

Coverage Status

Coverage increased (+0.07%) to 92.189% when pulling 58782ea on #250 into fa455e3 on master.

…ddressExtraction

Move AddressExtraction out of BlockFetcher because it is used in the
InternalTransactionFetcher and will be used in the
PendingTransactionFetcher.
Pending transactions have nil for "blockNumber" and "blockHash", which
broke `quantity_to_integer`, so for those keys, check if `nil` and pass
through.  For others, check that the value is not `nil` as it gives
a better error message with both the key and value instead of just the
nil value passed to quantity_to_integer as happened before.
Rename Indexer.BlockFetcher.monitor_task to Indexer.start_monitor, so
that all fetchers can use it.
from(
t in Transaction,
# exclude pending transactions
where: not is_nil(t.block_hash) and is_nil(t.internal_transactions_indexed_at),
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems important, wdyt of including a test for it?

Copy link
Contributor

@igorffs igorffs left a comment

Choose a reason for hiding this comment

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

Nice commits and description, really helpful for reviewing. ❤️ ✨

LGMT 🐑 🇮🇹

Parity doesn't support asking for the internal transactions of pending
transactions.
Not including timestamps is a developer error, so it should be caught as
soon as possible to allow the code to be fixed in development.
Because the repository transaction for a pending
`Explorer.Chain.Transaction`s could `COMMIT` after the repository
transaction for that same transaction being collated into a block,
writers, it is recommended to use `:nothing` for pending transactions
and `:replace_all` for collated transactions, so that collated
transactions win.
Use `parity_pendingTransactions` JSONRPC to get the pending transactions
and import them.  If they conflict with pre-existing transactions, then
the previous transactions win, under the assumption that sometimes
pending transactions repository transaction will COMMIT after the the
realtime index has COMMITed that same transaction being validated as the
timeout for transactions is 60 seconds while the block rate is faster
than that.
@KronicDeth KronicDeth merged commit 5f2e845 into master Jun 7, 2018
@KronicDeth KronicDeth deleted the #250 branch June 7, 2018 14:29
@ghost ghost removed the in progress label Jun 7, 2018
shellteo pushed a commit to shellteo/blockscout that referenced this pull request Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working enhancement New feature or request team: architecture
Development

Successfully merging this pull request may close these issues.

Index pending transactions
3 participants