-
Couldn't load subscription status.
- Fork 63
LW-11425 Misc fixes #1467
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
LW-11425 Misc fixes #1467
Conversation
2fe582a to
90cc07a
Compare
|
90cc07a to
a0e1351
Compare
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/DbSyncChainHistoryProvider.ts
Show resolved
Hide resolved
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/ChainHistoryBuilder.ts
Show resolved
Hide resolved
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
Show resolved
Hide resolved
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.
Just some minor/spelling change requests, feel free to squash those (no need for fixups)
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/ChainHistoryBuilder.ts
Outdated
Show resolved
Hide resolved
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.
As usual, great work @iccicci !
Love the split of changes into commits too 👏
c7707dd
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! 💪
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!
… providing transactions
…while providing transactions
…ol registration certificates
c7707dd to
764ec3d
Compare
Context
I noticed a problem about certificates type; while fixing it I noticed some other issues... while working on them I discovered some more else... that chain originated this misc fixes PR.
Discovered issues.
Cardano.CredentialType.KeyHashwhile building certificates.collateralReturn: it's useless to sort an up to 1 element array.nullforpoolParameterswhile buildingPoolRegistrationCertificates; this makesgetCertificatesByTypeto not pick given certificates.collateralReturnas it was an output but assets for it are stored inmulti_assets_descrcolumn not inma_tx_outtable.Proposed Solution
stakeCredentialdirectly fromcertModel.address.Cardano.CertificateType.StakeRegistrationwill be deprecated in favor ofCardano.CertificateType.Registrationthe test can be simplified to check the entire certificate.index(which is correct only foroutputs) while producinginputs. This result in a slight optimization as well.collateralsas well: it seems db-sync inserts them in a random order: we can't produce them with the original order. Removed the same sort as perinputsto apply the same slight optimization.collateralReturn.Cardano.PoolParametersfetching from DB the required data to fix the issue (therewardAccount) plus those data already loaded by the DB but not fetched.ownersandrelayswould be required as well. I considered that a new feature implementation and not handled in this fix PR.ChainHistoryBuilder.queryMultiAssetsByTxOutasma_tx_out.tx_outin case of fetchingcollateralReturnas it refers explicitly totx_outtable and not tocollateral_tx_outtable.collateralReturnwould be required to fetch and parsecollateral_tx_out.multi_assets_descrcolumn. I considered that a new feature implementation and not handled in this fix PR.Important Changes Introduced
yarn tx-cbor <txId>is enough, the script works regardless from local ports configuration (without the need to forward it given configuration).