Skip to content

LW-9675 conway era fixes #1480

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 20, 2024
Merged

LW-9675 conway era fixes #1480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
348c810
fix(cardano-services): refine update commitee proposal procedures acc…
iccicci Sep 18, 2024
71f6de7
fix(cardano-services): refine redeemer purpose accordingly new db-syn…
iccicci Sep 18, 2024
80b8da5
fix(cardano-services): resign commitee certificate
iccicci Sep 18, 2024
8f1ca36
chore(cardano-services): fix inline doc syntax
iccicci Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 9 additions & 25 deletions packages/cardano-services/src/ChainHistory/DbSyncChainHistory/ChainHistoryBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// cSpell:ignore descr
// cSpell:ignore descr timelock
Copy link
Member

Choose a reason for hiding this comment

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

Please remove all cSpell comments or set it up as a check in the repo

Copy link
Collaborator Author

@iccicci iccicci Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

cSpell is a VSCode extension. What is the problem?
Those who have it installed, can get benefit from it; those who don't, are not impacted by the comments.

Copy link
Member

Choose a reason for hiding this comment

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

Problem is that it's specific to your dev environment. These comments don't make sense for the repo in general. My suggestion here is to either:

  • add a "cspell" npm script and run it in CI
  • or don't commit those comments

Copy link
Collaborator Author

@iccicci iccicci Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I understand it's relative to my dev environment, but I can't see which problem those comments are originating.
Yes, it could be a good idea to add a repository level additional check, but it is OT from this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Ok let's make a deal: keep it but follow-up with a new PR to add this check :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The comments can be removed altogether.
Workspace ignored words are stored in .vscode/settings.json, and the .vscode folder is gitignored.
image

Copy link
Collaborator Author

@iccicci iccicci Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, and that is what I was doing, but that means every developer need to apply the same ignore by themselves... multiple times, on each distinct development environment they use.


import * as Queries from './queries';
import {
Expand Down Expand Up @@ -60,6 +60,7 @@ import orderBy from 'lodash/orderBy.js';

const {
CredentialType,
FractionUtils,
GovernanceActionType,
NetworkId: { Mainnet, Testnet },
RewardAccount,
Expand All @@ -77,26 +78,15 @@ const mapWithdrawals: (source: [{ credential: DbSyncCredential; network: string
rewardAccount: Cardano.RewardAccount;
coin: Cardano.Lovelace;
} = ([{ credential, network }, coin]) => ({
// Due to https://github.com/IntersectMBO/cardano-db-sync/issues/1614
// this will be source of NOT CORRECT AMOUNT for amounts greater than
// Number.MAX_SAFE_INTEGER (most likely, never).
// In case such amount will be present in some networks before db-sync solve the
// issue, we need to query the amount from treasury_withdrawal table.
// In case the issue is solved before we need to touch this again, the next line
// will work as well and we just need to remove this comment.
coin: BigInt(coin),
rewardAccount: RewardAccount.fromCredential(
credentialFromDbSync(credential),
network === 'Mainnet' ? Mainnet : Testnet
)
});

// eslint-disable-next-line sonarjs/cognitive-complexity, complexity
const getGovernanceAction = ({
denominator,
description,
numerator
}: ProposalProcedureModel): Cardano.GovernanceAction => {
// eslint-disable-next-line complexity, @typescript-eslint/no-explicit-any
const getGovernanceAction = (description: any): Cardano.GovernanceAction => {
const { contents, tag } = description;
const governanceActionId =
contents && contents[0] ? { actionIndex: contents[0].govActionIx, id: contents[0].txId } : null;
Expand All @@ -115,10 +105,7 @@ const getGovernanceAction = ({
case 'InfoAction':
return { __typename: GovernanceActionType.info_action };

case 'NewCommittee':
// LW-9675
if (typeof contents[3] !== 'number') throw new Error('New db-sync version detected: ref LW-9675');

case 'UpdateCommittee':
return {
__typename: GovernanceActionType.update_committee,
governanceActionId,
Expand All @@ -128,11 +115,8 @@ const getGovernanceAction = ({
epoch: value as Cardano.EpochNo
}))
),
membersToBeRemoved: new Set((contents[1] as DbSyncCredential[]).map(credentialFromDbSync)),
newQuorumThreshold: {
denominator: Number.parseInt(denominator!, 10),
numerator: Number.parseInt(numerator!, 10)
}
membersToBeRemoved: new Set(contents[1].map(credentialFromDbSync)),
newQuorumThreshold: typeof contents[3] === 'number' ? FractionUtils.toFraction(contents[3]) : contents[3]
};

case 'NewConstitution':
Expand Down Expand Up @@ -385,7 +369,7 @@ export class ChainHistoryBuilder {
const result = new Map<Cardano.TransactionId, Cardano.ProposalProcedure[]>();

for (const row of rows) {
const { data_hash, deposit, tx_id, url, view } = row;
const { data_hash, deposit, description, tx_id, url, view } = row;
const txId = tx_id.toString('hex') as Cardano.TransactionId;

const actions = (() => {
Expand All @@ -399,7 +383,7 @@ export class ChainHistoryBuilder {
actions.push({
anchor: mapAnchor(url, data_hash.toString('hex'))!,
deposit: BigInt(deposit),
governanceAction: getGovernanceAction(row),
governanceAction: getGovernanceAction(description),
rewardAccount: Cardano.RewardAccount(view)
});
}
Expand Down
6 changes: 3 additions & 3 deletions packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ const stubRedeemerData = Buffer.from('not implemented');
const redeemerPurposeMap: Record<RedeemerModel['purpose'], Cardano.RedeemerPurpose> = {
cert: Cardano.RedeemerPurpose.certificate,
mint: Cardano.RedeemerPurpose.mint,
proposing: Cardano.RedeemerPurpose.propose,
propose: Cardano.RedeemerPurpose.propose,
reward: Cardano.RedeemerPurpose.withdrawal,
spend: Cardano.RedeemerPurpose.spend,
voting: Cardano.RedeemerPurpose.vote
vote: Cardano.RedeemerPurpose.vote
};

const mapRedeemerPurpose = (purpose: RedeemerModel['purpose']): Cardano.RedeemerPurpose =>
Expand Down Expand Up @@ -351,7 +351,7 @@ export const mapCertificate = (
if (isResignCommitteeColdCertModel(certModel))
return {
__typename: Cardano.CertificateType.ResignCommitteeCold,
anchor: mapAnchor(certModel.url, certModel.data_hash),
anchor: mapAnchor(certModel.url, certModel.data_hash.toString('hex')),
cert_index: certModel.cert_index,
coldCredential: {
hash: certModel.cold_key.toString('hex') as unknown as Crypto.Hash28ByteBase16,
Expand Down
7 changes: 3 additions & 4 deletions packages/cardano-services/src/ChainHistory/DbSyncChainHistory/queries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// cSpell:ignore serialised

export const DB_MAX_SAFE_INTEGER = 2_147_483_647;

const selectTxInput = (collateral?: boolean) => `
Expand Down Expand Up @@ -224,14 +226,11 @@ export const findProposalProceduresByTxIds = `
ga.description,
va.url,
va.data_hash,
sa.view,
quorum_numerator AS numerator,
quorum_denominator AS denominator
sa.view
FROM tx
JOIN gov_action_proposal AS ga ON tx.id = ga.tx_id
JOIN voting_anchor AS va ON voting_anchor_id = va.id
JOIN stake_address AS sa ON ga.return_address = sa.id
LEFT JOIN new_committee AS nc ON gov_action_proposal_id = ga.id
WHERE tx.id = ANY($1)
ORDER BY ga.index`;

Expand Down
9 changes: 4 additions & 5 deletions packages/cardano-services/src/ChainHistory/DbSyncChainHistory/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// cSpell:ignore serialised timelock

import { Cardano } from '@cardano-sdk/core';
import { CostModelsParamModel } from '../../NetworkInfo/DbSyncNetworkInfoProvider/types';

Expand Down Expand Up @@ -105,7 +107,7 @@ export interface WithdrawalModel {

export interface RedeemerModel {
index: number;
purpose: 'cert' | 'mint' | 'spend' | 'reward' | 'voting' | 'proposing';
purpose: 'cert' | 'mint' | 'spend' | 'reward' | 'vote' | 'propose';
Copy link
Member

Choose a reason for hiding this comment

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

Are db-sync master branch schema docs incorrect?
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IDK, I didn't checked...

Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for 71f6de7 commit then? Which db-sync version it is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

db-sync version is 13.5.0.1
I taken the values directly from the DB

Copy link
Member

@mkazlauskas mkazlauskas Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

13.5.0.1 tagged commit also has voting and proposing in docs. Let's report to db-sync devs

script_hash: Buffer;
unit_mem: string;
unit_steps: string;
Expand Down Expand Up @@ -139,9 +141,6 @@ export interface ProposalProcedureModel {
tx_id: Buffer;
url: string;
view: string;
// LW-9675
numerator?: string;
denominator?: string;
}

export interface CertificateModel {
Expand Down Expand Up @@ -235,7 +234,7 @@ export interface ResignCommitteeColdCertModel extends CertificateModel {
cold_key: Buffer;
cold_key_has_script: boolean;
url: string;
data_hash: string;
data_hash: Buffer;
}

export interface TxIdModel {
Expand Down
4 changes: 2 additions & 2 deletions packages/cardano-services/src/ChainHistory/DbSyncChainHistory/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export const applyPagination = (ids: Cardano.TransactionId[], { startAt, limit }
* certificates_.
*
* Example: given an array of `reg_cert` and an array of `vote_deleg_cert`, the result is an array of:
* - all input `reg_cert` which can't be neither `vote_reg_deleg_cert` nor `stake_vote_reg_deleg_cert,
* - all input `vote_deleg_cert` which can't be neither `vote_reg_deleg_cert` nor `stake_vote_reg_deleg_cert,
* - all input `reg_cert` which can be neither `vote_reg_deleg_cert` nor `stake_vote_reg_deleg_cert`,
* - all input `vote_deleg_cert` which can be neither `vote_reg_deleg_cert` nor `stake_vote_reg_deleg_cert`,
* - the array of `vote_reg_deleg_cert` (it includes also the partials `stake_vote_reg_deleg_cert`)
*
* @param certs1 the first certificates array
Expand Down
4 changes: 2 additions & 2 deletions packages/cardano-services/test/ChainHistory/DbSyncChainHistoryProvider/mappers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,8 @@ describe('chain history mappers', () => {
['mint' as const, Cardano.RedeemerPurpose.mint],
['cert' as const, Cardano.RedeemerPurpose.certificate],
['reward' as const, Cardano.RedeemerPurpose.withdrawal],
['voting' as const, Cardano.RedeemerPurpose.vote],
['proposing' as const, Cardano.RedeemerPurpose.propose]
['vote' as const, Cardano.RedeemerPurpose.vote],
['propose' as const, Cardano.RedeemerPurpose.propose]
])("maps '%p' redeemer", (dbSyncRedeemerPurpose, sdkRedeemerPurpose) => {
const result = mappers.mapRedeemer({ ...redeemerModel, purpose: dbSyncRedeemerPurpose });
expect(result).toEqual<Cardano.Redeemer>({
Expand Down
Loading