-
Notifications
You must be signed in to change notification settings - Fork 45
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
Feature/rewrite address api #126
Conversation
Codecov Report
@@ Coverage Diff @@
## master #126 +/- ##
==========================================
- Coverage 60.18% 60.11% -0.07%
==========================================
Files 205 205
Lines 5771 5779 +8
Branches 276 263 -13
==========================================
+ Hits 3473 3474 +1
- Misses 2298 2305 +7
Continue to review full report at Codecov.
|
if(stateSettings.txTypeAccountTxIds){ | ||
val txNum = state.txTypeAccTxLengths(TransactionType.LeaseTransaction, a) | ||
val txIds = state.txTypeAccountTxIds(TransactionType.LeaseTransaction, a, txNum, 0) | ||
if (txIds._2 != null && txIds._2.size > 0) { |
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.
avoid to use null in scala
) | ||
res match { | ||
case Some(tx) => complete(Json.arr(JsArray(tx))) | ||
case None => complete(StatusCodes.NotFound -> Json.obj("message" -> "Address has no transaction")) |
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.
no txs does not mean NotFound
if(stateSettings.txTypeAccountTxIds){ | ||
val txNum = state.txTypeAccTxLengths(TransactionType.LeaseTransaction, a) | ||
val txIds = state.txTypeAccountTxIds(TransactionType.LeaseTransaction, a, txNum, 0) | ||
val res = Option(txIds._2) |
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.
Option is useless
val txNum = state.txTypeAccTxLengths(TransactionType.LeaseTransaction, a) | ||
val txIds = state.txTypeAccountTxIds(TransactionType.LeaseTransaction, a, txNum, 0) | ||
val res = Option(txIds._2) | ||
.map(_.flatMap(state.transactionInfo) |
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.
????
val txIds = state.txTypeAccountTxIds(TransactionType.LeaseTransaction, a, txNum, 0) | ||
val res = Option(txIds._2) | ||
.map(_.flatMap(state.transactionInfo) | ||
.map(a => (a._1,a._2,a._2.transaction)) |
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.
do not copy the codes below. result of state.activeLeases()
is already filtered by lease status
.filter(a => state.isLeaseActive(a._3.asInstanceOf[LeaseTransaction])) | ||
.collect{ | ||
case (h:Int, tx:ProcessedTransaction, lt:LeaseTransaction) | ||
if EllipticCurve25519Proof.fromBytes(lt.proofs.proofs.head.bytes.arr).toOption.get.publicKey.address == address => |
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.
useless filter
val txNum = state.txTypeAccTxLengths(TransactionType.LeaseTransaction, a) | ||
val txIds = state.txTypeAccountTxIds(TransactionType.LeaseTransaction, a, txNum, 0) | ||
complete(Json.arr(JsArray(txIds._2 | ||
.flatMap(state.transactionInfo) |
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.
txIds._2.flatMap(state.transactionInfo)
is already implemented in txTypeAccountTransactions
.flatMap(state.transactionInfo) | ||
.map(a => (a._1, a._2, a._2.transaction)) | ||
.filter(a => state.isLeaseActive(a._3.asInstanceOf[LeaseTransaction])) | ||
.map(a => (processedTxToExtendedJson(a._2) + ("height" -> JsNumber(a._1)))) |
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.
can refactored to one .map
complete(Json.arr(JsArray(txIds._2 | ||
.flatMap(state.transactionInfo) | ||
.map(a => (a._1, a._2, a._2.transaction)) | ||
.filter(a => state.isLeaseActive(a._3.asInstanceOf[LeaseTransaction])) |
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.
call isLeaseActive
in loop is not in one business transaction. need read lock here
PR Details
Change implementation of getting txId
Description
In original code, transaction id is got by traversing whole state. This version use transaction type and address to get transaction id directly
Related Issue
#113
Motivation and Context
Increase performance of address api
How Has This Been Tested
Unit test has passed
Types of changes
Checklist