-
Notifications
You must be signed in to change notification settings - Fork 27
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
CORE-18698 Store and find filtered transactions in the database #5485
CORE-18698 Store and find filtered transactions in the database #5485
Conversation
...dger-persistence/src/main/kotlin/net/corda/ledger/persistence/utxo/UtxoPersistenceService.kt
Outdated
Show resolved
Hide resolved
I was wondering whether we need new modules for filtered transactions? Couldn't they be moved into |
I think we should split out moving the code around and get that merged first so it's easier to read the behaviour changes. |
AND utmp.group_idx = :groupIndex""" | ||
WHERE utmp.transaction_id IN (:transactionIds) | ||
AND utc.leaf_idx IN ( | ||
SELECT utmpl.leaf_index |
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 you get input from @ifrankrui on the best way to write those queries?
...istence/src/main/kotlin/net/corda/ledger/persistence/utxo/impl/UtxoPersistenceServiceImpl.kt
Outdated
Show resolved
Hide resolved
...istence/src/main/kotlin/net/corda/ledger/persistence/utxo/impl/UtxoPersistenceServiceImpl.kt
Outdated
Show resolved
Hide resolved
…me-os into nandor/CORE-18698/persist-and-find-ftx # Conflicts: # gradle.properties
…me-os into nandor/CORE-18698/persist-and-find-ftx # Conflicts: # gradle.properties
Jenkins build for PR 5485 build 32 Build Successful: |
…me-os into nandor/CORE-18698/persist-and-find-ftx # Conflicts: # components/ledger/ledger-persistence/src/main/kotlin/net/corda/ledger/persistence/utxo/UtxoPersistenceService.kt # components/ledger/ledger-utxo-flow/src/main/kotlin/net/corda/ledger/utxo/flow/impl/persistence/UtxoLedgerPersistenceService.kt # components/ledger/ledger-utxo-flow/src/main/kotlin/net/corda/ledger/utxo/flow/impl/persistence/UtxoLedgerPersistenceServiceImpl.kt
…me-os into nandor/CORE-18698/persist-and-find-ftx
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 added a few small things potentially for a later PR.
listOf(0, 1), | ||
emptyList() | ||
persistenceService.persistFilteredTransactions( | ||
mapOf(filteredTransactionToStore to emptyList()), |
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 would be good to verify the signature persistence's execution path.
But that can happen in a separate PR/ticket.
signedTransaction.wireTransaction, | ||
componentGroupFilterParameters = listOf( | ||
ComponentGroupFilterParameters.AuditProof( | ||
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.
UtxoComponentGroup.OUTPUTS_INFO.ordinal would be a bit nicer instead of the constant.
Or potentially using the LedgerService.filterSignedTransaction
to replace the creation steps. (if that works in integration tests.)
) | ||
|
||
// 6. Map the transaction id to the filtered transaction object and signatures | ||
filteredTransaction.id.toString() to Pair(filteredTransaction, ftxDto.signatures) |
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.
transactionId
is already the same in string.
import net.corda.v5.crypto.extensions.merkle.MerkleTreeHashDigestProvider | ||
import net.corda.v5.crypto.merkle.HashDigestConstants | ||
import net.corda.v5.ledger.common.transaction.TransactionMetadata | ||
import java.util.* |
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.
Is detekt OK with wildcard import?
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.
Re-approve
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.
LGTM (flow related code)
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 2 New issues |
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.
LGTM
This PR contains the following changes:
ledger-filtered-transaction-api
andledger-filtered-transaction
. This is needed because the common flow module didn't allow us to use these in the persistence worker.MerkleProofInternal
which has amerge
function on it so we don't have to useMerkleProofImpl
everywhere.UtxoFilteredTransactionInternal
so we can access the itsFilteredTransaction
.FilteredTransaction
now exposesprivacySalt
UtxoLedgerPersistenceService
persistFilteredTransactions
andfindFilteredTransactions
on theUtxoPersistenceService
API (persistence worker side)Draft PR for the requests that I used for testing:
corda/corda-api#1456
#5490