Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

Pending #571

Merged
merged 83 commits into from
Dec 15, 2020
Merged

Pending #571

merged 83 commits into from
Dec 15, 2020

Conversation

araskachoi
Copy link
Contributor

@araskachoi araskachoi commented Oct 9, 2020

Closes: #XXX

Description

[x]eth_getBalance
[x]eth_getTransactionCount
[x]eth_getBlockTransactionCountByNumber
[]eth_call
[]eth_estimateGas
[x]eth_getBlockByNumber

  • if querying "pending", returns information about the next pending block
  • timestamp will be set to: 0001-01-01 00:00:00 +0000 UTC
  • size will be set to: 0x

[x]eth_getTransactionByHash

  • querying a pending transaction hash will give blockNumber = nil

[x]eth_getTransactionByBlockNumberAndIndex

  • pending query will return blockhash = nil and blocknumber = nil

[x]eth_sendTransaction


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2020

This pull request introduces 1 alert when merging 7b49761 into d274c76 - view on LGTM.com

new alerts:

  • 1 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2020

This pull request introduces 1 alert when merging f6550be into d274c76 - view on LGTM.com

new alerts:

  • 1 for Incorrect conversion between integer types

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request introduces 1 alert when merging 04486f4 into be09a6e - view on LGTM.com

new alerts:

  • 1 for Incorrect conversion between integer types

araskachoi and others added 10 commits December 15, 2020 10:31
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

few minor comments

tests/tests-pending/rpc_pending_test.go Outdated Show resolved Hide resolved
tests/tests-pending/rpc_pending_test.go Outdated Show resolved Hide resolved
tests/utils.go Outdated Show resolved Hide resolved
araskachoi and others added 6 commits December 15, 2020 11:43
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Great work! 👍

@mergify mergify bot merged commit cb96bc4 into development Dec 15, 2020
@mergify mergify bot deleted the pending branch December 15, 2020 19:52
if err := msg.ValidateBasic(); err != nil {
return nil, err
// convert the pending transactions into ethermint msgs
if blockNum == rpctypes.PendingBlockNumber {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the pending transaction put in here? My understanding is that only the execution result of one transaction is needed here

}

// number of pending txs queried from the mempool
unconfirmedTxs, err := api.clientCtx.Client.UnconfirmedTxs(1000)
Copy link
Contributor

@summerpro summerpro Dec 17, 2020

Choose a reason for hiding this comment

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

What if the actual tx quantity is more than 1000?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's highly unlikely (for now) as on the Cosmos Hub there are 1 or 2 avg txs per block.

@fedekunze fedekunze mentioned this pull request Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge rpc Ethereum JSON-RPC related issues and PRs Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bad tx returns non-error
4 participants