-
Notifications
You must be signed in to change notification settings - Fork 161
Conversation
This pull request introduces 1 alert when merging 7b49761 into d274c76 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging f6550be into d274c76 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 04486f4 into be09a6e - view on LGTM.com new alerts:
|
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>
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.
few minor comments
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.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.
Great work! 👍
if err := msg.ValidateBasic(); err != nil { | ||
return nil, err | ||
// convert the pending transactions into ethermint msgs | ||
if blockNum == rpctypes.PendingBlockNumber { |
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.
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) |
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.
What if the actual tx quantity is more than 1000?
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.
It's highly unlikely (for now) as on the Cosmos Hub there are 1 or 2 avg txs per block.
Closes: #XXX
Description
[x]eth_getBalance
[x]eth_getTransactionCount
[x]eth_getBlockTransactionCountByNumber
[]eth_call
[]eth_estimateGas
[x]eth_getBlockByNumber
0001-01-01 00:00:00 +0000 UTC
0x
[x]eth_getTransactionByHash
blockNumber = nil
[x]eth_getTransactionByBlockNumberAndIndex
blockhash = nil
andblocknumber = nil
[x]eth_sendTransaction
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerFor admin use:
WIP
,R4R
,docs
, etc)