-
Notifications
You must be signed in to change notification settings - Fork 3
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
store inputs #6
store inputs #6
Conversation
…s and dataInputs to db
app/dao/ExtractionResultDAO.scala
Outdated
extends DbUtils with HasDatabaseConfigProvider[JdbcProfile] { | ||
|
||
import profile.api._ | ||
/** | ||
* store data into db as transactionally. |
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.
Unfinished description
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.
Change naming and descriptions
app/dao/ExtractionResultDAO.scala
Outdated
* @param extractionResult : ExtractionResultModel extracted data | ||
* @param spentTrackedInputs : ExtractionInputResultModel extracted spent Tracked Inputs | ||
*/ | ||
def storeBaseOnInputs(spentTrackedInputs: Seq[ExtractionInputResultModel]): Unit = { |
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.
Sounds like inputs for tracked outputs being spent passed, while in fact all the inputs in the block. Please fix descriptions and naming.
Inputs filtering can be very inefficient, can we consider using and storing a Bloom filter maybe?
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.
I think according to the database queries, this is the only way to use Bloom filter, If you have another comment please share
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 seems Bloom filter is not doing doing much here, also, after checking with mightContain you need to check for false positives. Let's revert ?
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.
Revert on which? the filter or check false positives for Bloom filter?
I think revert on the filter is better!
Related to #4
Some Changes:
dataInputs
DAO and functionality for store them