Skip to content
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

feat(tx-construction): throw when deregistering reward accounts with … #1547

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions packages/tx-construction/src/tx-builder/TxBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
import { SelectionSkeleton } from '@cardano-sdk/input-selection';
import { contextLogger, deepEquals } from '@cardano-sdk/util';
import { createOutputValidator } from '../output-validation';
import { ensureNoDeRegistrationsWithRewardsLocked } from './ensureNoDeRegistrationsWithRewardsLocked';
import { initializeTx } from './initializeTx';
import { lastValueFrom } from 'rxjs';
import { poll } from '@cardano-sdk/util-rxjs';
Expand Down Expand Up @@ -639,6 +640,8 @@ export class GenericTxBuilder implements TxBuilder {
rewardAccountsWithWeights.set(rewardAccount.address, weight);
}

ensureNoDeRegistrationsWithRewardsLocked(availableRewardAccounts);

// Deregister stake keys no longer needed
this.#logger.debug(`De-registering ${availableRewardAccounts.length} stake keys`);
for (const rewardAccount of availableRewardAccounts) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { DeRegistrationsWithRewardsLocked } from './types';
import { hasCorrectVoteDelegation } from './hasCorrectVoteDelegation';
import type { RewardAccountWithPoolId } from '../types';

export const ensureNoDeRegistrationsWithRewardsLocked = (rewardAccountsToBeDeRegistered: RewardAccountWithPoolId[]) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the nitpicking. Could you reduce this type too?

- export const ensureNoDeRegistrationsWithRewardsLocked = (rewardAccountsToBeDeRegistered: RewardAccountWithPoolId[]) => {
+ export const ensureNoDeRegistrationsWithRewardsLocked = (rewardAccountsToBeDeRegistered: Pick<RewardAccountWithPoolId, 'dRepDelegatee' | 'rewardBalance'>[]) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would not reduce it here. This function uses hasCorrectVoteDelegation function which has that type already reduced so reducing it here would create an implicit relation between them as the ensureNoDeRegistrationsWithRewardsLocked would need to pick dRepDelegatee despite not using it

const rewardAccountsWithLockedRewards = rewardAccountsToBeDeRegistered.filter(
(rewardAccountWithPoolId) =>
rewardAccountWithPoolId.rewardBalance > 0n && !hasCorrectVoteDelegation(rewardAccountWithPoolId)
);

if (rewardAccountsWithLockedRewards.length > 0) {
throw new DeRegistrationsWithRewardsLocked(rewardAccountsWithLockedRewards);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Cardano } from '@cardano-sdk/core';
import { RewardAccountWithPoolId } from '../types';

export const hasCorrectVoteDelegation = ({
dRepDelegatee
}: Pick<RewardAccountWithPoolId, 'dRepDelegatee'>): boolean => {
const drep = dRepDelegatee?.delegateRepresentative;
return !!drep && (!Cardano.isDrepInfo(drep) || drep.active);
};
14 changes: 3 additions & 11 deletions packages/tx-construction/src/tx-builder/initializeTx.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,20 @@
import { StaticChangeAddressResolver, roundRobinRandomImprove } from '@cardano-sdk/input-selection';

import { Bip32Account, SignTransactionContext, util } from '@cardano-sdk/key-management';
import { Cardano, Serialization } from '@cardano-sdk/core';
import { Ed25519KeyHashHex } from '@cardano-sdk/crypto';
import { GreedyTxEvaluator } from './GreedyTxEvaluator';
import { InitializeTxProps, InitializeTxResult, RewardAccountWithPoolId } from '../types';
import { RedeemersByType, defaultSelectionConstraints } from '../input-selection';
import { StaticChangeAddressResolver, roundRobinRandomImprove } from '@cardano-sdk/input-selection';
import { TxBuilderDependencies } from './types';
import { createPreInputSelectionTxBody, includeChangeAndInputs } from '../createTransactionInternals';
import { ensureValidityInterval } from '../ensureValidityInterval';
import { hasCorrectVoteDelegation } from './hasCorrectVoteDelegation';

const dRepPublicKeyHash = async (addressManager?: Bip32Account): Promise<Ed25519KeyHashHex | undefined> =>
addressManager && (await (await addressManager.derivePublicKey(util.DREP_KEY_DERIVATION_PATH)).hash()).hex();

const DREP_REG_REQUIRED_PROTOCOL_VERSION = 10;

const isActive = (drepDelegatee?: Cardano.DRepDelegatee): boolean => {
const drep = drepDelegatee?.delegateRepresentative;
if (!drep || (Cardano.isDrepInfo(drep) && drep.active === false)) {
return false;
}
return true;
};

/**
* Filters and transforms reward accounts based on current protocol version and reward balance.
*
Expand All @@ -45,7 +37,7 @@ const getWithdrawals = (
accounts
.filter(
(account) =>
(version.major >= DREP_REG_REQUIRED_PROTOCOL_VERSION ? isActive(account.dRepDelegatee) : true) &&
(version.major >= DREP_REG_REQUIRED_PROTOCOL_VERSION ? hasCorrectVoteDelegation(account) : true) &&
!!account.rewardBalance
)
.map(({ rewardBalance: quantity, address: stakeAddress }) => ({
Expand Down
20 changes: 19 additions & 1 deletion packages/tx-construction/src/tx-builder/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
import { Cardano, Handle, HandleProvider, HandleResolution } from '@cardano-sdk/core';
import { CustomError } from 'ts-custom-error';
import { Hash32ByteBase16 } from '@cardano-sdk/crypto';
import { InitializeTxWitness, TxBodyPreInputSelection, TxBuilderProviders } from '../types';
import { HexBlob } from '@cardano-sdk/util';
import { InitializeTxWitness, RewardAccountWithPoolId, TxBodyPreInputSelection, TxBuilderProviders } from '../types';
import { InputSelectionError, InputSelector, SelectionSkeleton } from '@cardano-sdk/input-selection';
import { Logger } from 'ts-log';
import { OutputBuilderValidator } from './OutputBuilder';
Expand Down Expand Up @@ -56,6 +57,23 @@ export class InsufficientRewardAccounts extends CustomError {
}
}

export type DataOfKeyWithLockedRewards = {
cbor?: HexBlob;
key: Cardano.RewardAccount;
};

export class DeRegistrationsWithRewardsLocked extends CustomError {
keysWithLockedRewards: DataOfKeyWithLockedRewards[];

public constructor(rewardAccounts: Pick<RewardAccountWithPoolId, 'address'>[]) {
super('Tried to de-register reward accounts that have not delegated vote but have rewards');
this.keysWithLockedRewards = rewardAccounts.map(({ address }) => ({
cbor: Cardano.Address.fromString(address)?.toBytes(),
key: address
}));
}
}

/** New stake keys derived for multi-delegation were not found in the rewardAccounts provider */
export class OutOfSyncRewardAccounts extends CustomError {
public constructor(rewardAccounts: Cardano.RewardAccount[]) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable sonarjs/no-duplicate-string */
import * as Crypto from '@cardano-sdk/crypto';
import { AddressType, Bip32Account, GroupedAddress, InMemoryKeyAgent, util } from '@cardano-sdk/key-management';
import { Cardano, setInConwayEra } from '@cardano-sdk/core';
import { Cardano, DRepInfo, setInConwayEra } from '@cardano-sdk/core';
import {
DeRegistrationsWithRewardsLocked,
GenericTxBuilder,
OutOfSyncRewardAccounts,
OutputValidation,
Expand Down Expand Up @@ -40,12 +41,14 @@ const inputResolver: Cardano.InputResolver = {

/** Utility factory for tests to create a GenericTxBuilder with mocked dependencies */
const createTxBuilder = async ({
adjustRewardAccount = (r) => r,
stakeDelegations,
numAddresses = stakeDelegations.length,
useMultiplePaymentKeys = false,
rewardAccounts,
keyAgent
}: {
adjustRewardAccount?: (rewardAccountWithPoolId: RewardAccountWithPoolId, index: number) => RewardAccountWithPoolId;
stakeDelegations: {
credentialStatus: Cardano.StakeCredentialStatus;
poolId?: Cardano.PoolId;
Expand Down Expand Up @@ -88,13 +91,21 @@ const createTxBuilder = async ({
// This would normally be done by the wallet.delegation.rewardAccounts
.map<RewardAccountWithPoolId>(({ rewardAccount: address }, index) => {
const { credentialStatus, poolId, deposit } = stakeDelegations[index] ?? {};
return {
address,
credentialStatus: credentialStatus ?? Cardano.StakeCredentialStatus.Unregistered,
rewardBalance: mocks.rewardAccountBalance,
...(poolId ? { delegatee: { nextNextEpoch: { id: poolId } } } : undefined),
...(deposit && { deposit })
};
return adjustRewardAccount(
{
address,
credentialStatus: credentialStatus ?? Cardano.StakeCredentialStatus.Unregistered,
dRepDelegatee: {
delegateRepresentative: {
__typename: 'AlwaysAbstain'
}
},
rewardBalance: mocks.rewardAccountBalance,
...(poolId ? { delegatee: { nextNextEpoch: { id: poolId } } } : undefined),
...(deposit && { deposit })
},
index
);
})
)
),
Expand Down Expand Up @@ -853,4 +864,131 @@ describe('TxBuilder/delegatePortfolio', () => {
expect(roundRobinRandomImprove).toHaveBeenCalled();
});
});

// eslint-disable-next-line sonarjs/cognitive-complexity
describe('DRep delegation', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very well written tests. Great coverage and easy to ready ❤️ 👏

const dRepInfo: DRepInfo = {
active: true,
amount: 9586n,
hasScript: false,
id: Cardano.DRepID('drep15cfxz9exyn5rx0807zvxfrvslrjqfchrd4d47kv9e0f46uedqtc')
};

const dRepDelegateeOptions = {
abstain: { __typename: 'AlwaysAbstain' },
activeDrep: dRepInfo,
inactiveDrep: { ...dRepInfo, active: false },
noConfidence: { __typename: 'AlwaysNoConfidence' },
none: undefined
} as const;

const createTxBuilderWithRewardBalance =
(rewardBalance: bigint) =>
(...delegateRepresentatives: (Cardano.DRepDelegatee['delegateRepresentative'] | undefined)[]) =>
createTxBuilder({
adjustRewardAccount: (rewardAccountWithPoolId, index) => {
const delegateRepresentative = delegateRepresentatives[index];
return {
...rewardAccountWithPoolId,
dRepDelegatee: delegateRepresentative ? { delegateRepresentative } : undefined,
rewardBalance: rewardBalance ?? rewardAccountWithPoolId.rewardBalance
};
},
keyAgent,
numAddresses: delegateRepresentatives.length,
stakeDelegations: []
});

describe('when reward accounts have no balance', () => {
it('can deregister all reward accounts no matter the drep delegation', async () => {
const drepValues = Object.values(dRepDelegateeOptions);
const createTxBuilderWithDreps = createTxBuilderWithRewardBalance(0n);
const txBuilderFactory = await createTxBuilderWithDreps(...drepValues);

await expect(txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect()).resolves.not.toThrow();
});
});

describe('when deregistered reward account has balance', () => {
const createTxBuilderWithDreps = createTxBuilderWithRewardBalance(7365n);

it('deregisters the key when it delegates to abstain', async () => {
const txBuilderFactory = await createTxBuilderWithDreps(dRepDelegateeOptions.abstain);
await expect(txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect()).resolves.not.toThrow();
});

it('deregisters the key when it delegates to no confidence', async () => {
const txBuilderFactory = await createTxBuilderWithDreps(dRepDelegateeOptions.noConfidence);
await expect(txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect()).resolves.not.toThrow();
});

it('deregisters the key when it delegates to active drep', async () => {
const txBuilderFactory = await createTxBuilderWithDreps(dRepDelegateeOptions.activeDrep);
await expect(txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect()).resolves.not.toThrow();
});

it('throws and DeRegistrationsWithRewardsLocked when it delegates to inactive drep', async () => {
const txBuilderFactory = await createTxBuilderWithDreps(dRepDelegateeOptions.inactiveDrep);
const [{ address: lockedRewardAccount }] = await txBuilderFactory.txBuilderProviders.rewardAccounts();

try {
await txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect();
throw new Error('TxBuilder supposed to throw an DeRegistrationsWithRewardsLocked error');
} catch (error) {
expect(error).toBeInstanceOf(DeRegistrationsWithRewardsLocked);
expect((error as DeRegistrationsWithRewardsLocked).keysWithLockedRewards[0]).toEqual({
cbor: Cardano.Address.fromString(lockedRewardAccount)?.toBytes(),
key: lockedRewardAccount
});
}
});

it('throws an DeRegistrationsWithRewardsLocked when it has no drep delegation', async () => {
const txBuilderFactory = await createTxBuilderWithDreps(dRepDelegateeOptions.none);
const [{ address: lockedRewardAccount }] = await txBuilderFactory.txBuilderProviders.rewardAccounts();

try {
await txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect();
throw new Error('TxBuilder supposed to throw an DeRegistrationsWithRewardsLocked error');
} catch (error) {
expect(error).toBeInstanceOf(DeRegistrationsWithRewardsLocked);
expect((error as DeRegistrationsWithRewardsLocked).keysWithLockedRewards[0]).toEqual({
cbor: Cardano.Address.fromString(lockedRewardAccount)?.toBytes(),
key: lockedRewardAccount
});
}
});
});

it('attaches data of all locked reward accounts to the DeRegistrationsWithRewardsLocked error', async () => {
const createTxBuilderWithDreps = createTxBuilderWithRewardBalance(7365n);
const txBuilderFactory = await createTxBuilderWithDreps(
dRepDelegateeOptions.none,
dRepDelegateeOptions.activeDrep,
dRepDelegateeOptions.inactiveDrep
);
const allrewardAccountsData = await txBuilderFactory.txBuilderProviders.rewardAccounts();
const lockedRewardAccountsData = allrewardAccountsData
// TXBuilder reverses them so doing the same here so it is more convenient here to test
.reverse()
// At the index 1 there is an activeDrep allowing to withdraw during de-registering
.filter((_, index) => index !== 1)
.map((r) => ({
cbor: Cardano.Address.fromString(r.address)?.toBytes(),
key: r.address
}));

try {
await txBuilderFactory.txBuilder.delegatePortfolio(null).build().inspect();
throw new Error('TxBuilder supposed to throw an DeRegistrationsWithRewardsLocked error');
} catch (error) {
expect(error).toBeInstanceOf(DeRegistrationsWithRewardsLocked);
for (const [index, keyWithLockedRewards] of (
error as DeRegistrationsWithRewardsLocked
).keysWithLockedRewards.entries()) {
expect(keyWithLockedRewards).toEqual(lockedRewardAccountsData[index]);
}
}
});
});
});
Loading