Skip to content

Conversation

@kapke
Copy link
Contributor

@kapke kapke commented Nov 20, 2020

Description

Move transaction history method to mantis_ namespace, and add couple of additional informations a requested by Mantis Wallet devs.

It's a high priority because it's quite important from wallet perspective.

Testing

  1. all unit tests should pass
  2. check if method complains about too large number of blocks within range
  3. check whether method finds transactions sent or received by given address (only direct sender or direct receiver is supported so far), example tx on mordor: https://blockscout.com/etc/mordor/tx/0x6249ce7bae7e6c57b08f4a9333bd335a9c001b648e2b62b30c4cea8e80fa9842/internal-transactions, it's a tx from faucet to my testing address

@kapke kapke added high priority This PRs should be reviewed and merged ASAP BREAKS API Affects services that interacts with APIs BREAKS CONFIG Affects the default configuration labels Nov 20, 2020
@kapke kapke requested review from lemastero and mmrozek November 20, 2020 22:09
@kapke kapke self-assigned this Nov 20, 2020
@kapke kapke force-pushed the etcm-135-tx-history-improvements branch from 48587d3 to b45d65f Compare November 20, 2020 22:10
@kapke kapke added the wallet label Nov 20, 2020
@kapke kapke changed the title [ETCM-135] Extract tx history logic into separate service [ETCM-135] Tx history improvements for wallet Nov 20, 2020
.concatMap { block =>
val getBlockReceipts = Task {
blockchain.getReceiptsByHash(block.hash).map(_.toVector).getOrElse(Vector.empty)
}.memoizeOnSuccess
Copy link
Contributor

Choose a reason for hiding this comment

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

memoizeOnSuccess won't work here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it will work on a per-block basis, won't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I missed it

Copy link
Contributor

@mmrozek mmrozek left a comment

Choose a reason for hiding this comment

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

LGTM


// scalastyle:off number.of.methods
object EthJsonMethodsImplicits extends JsonMethodsImplicits {
implicit val transactionResponseJsonEncoder: JsonEncoder[TransactionResponse] = Extraction.decompose(_)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@kapke kapke force-pushed the etcm-135-tx-history-improvements branch 2 times, most recently from 8b278fc to 60cfdc2 Compare November 25, 2020 11:48
@kapke kapke force-pushed the etcm-135-tx-history-improvements branch from 794d398 to 6c26ff1 Compare November 25, 2020 12:25
@kapke kapke merged commit adfa1f2 into develop Nov 25, 2020
@kapke kapke deleted the etcm-135-tx-history-improvements branch November 25, 2020 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BREAKS API Affects services that interacts with APIs BREAKS CONFIG Affects the default configuration high priority This PRs should be reviewed and merged ASAP wallet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants