Skip to content

Commit

Permalink
fix: use proper batch size for bulk payout
Browse files Browse the repository at this point in the history
  • Loading branch information
dnechay committed Jan 9, 2025
1 parent f061a87 commit 90708c3
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -486,5 +486,3 @@ export const CURSE_WORDS = [
'xrated',
'xxx',
];

export const ESCROW_BULK_MAX_COUNT = 100;
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const MOCK_ADDRESS_PAYOUT = {
};
const mockPayouts = [
MOCK_ADDRESS_PAYOUT,
...Array.from({ length: 100 }, (_value, index) => {
...Array.from({ length: 99 }, (_value, index) => {
const _index = index + 1;
return {
address: `0x${_index}`.padEnd(42, '0'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@ import { ServerConfigService } from '../../common/config/server-config.service';
import { EscrowCompletionRepository } from './escrow-completion.repository';
import { EscrowCompletionEntity } from './escrow-completion.entity';
import {
ESCROW_BULK_PAYOUT_MAX_ITEMS,
ChainId,
EscrowClient,
EscrowStatus,
OperatorUtils,
} from '@human-protocol/sdk';
import { calculateExponentialBackoffMs } from '../../common/utils/backoff';
import {
BACKOFF_INTERVAL_SECONDS,
ESCROW_BULK_MAX_COUNT,
} from '../../common/constants';
import { BACKOFF_INTERVAL_SECONDS } from '../../common/constants';
import { WebhookIncomingService } from '../webhook/webhook-incoming.service';
import { PayoutService } from '../payout/payout.service';
import { ReputationService } from '../reputation/reputation.service';
Expand Down Expand Up @@ -139,7 +137,7 @@ export class EscrowCompletionService {
*/
const payoutBatches = _.chunk(
_.orderBy(calculatedPayouts, 'address', 'asc'),
ESCROW_BULK_MAX_COUNT,
ESCROW_BULK_PAYOUT_MAX_ITEMS,
);

await Promise.all(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,3 +352,6 @@ class KVStoreKeys(Enum):
job_types = "job_types"
registration_needed = "registration_needed"
registration_instructions = "registration_instructions"


ESCROW_BULK_PAYOUT_MAX_ITEMS = 99
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ def get_w3_with_priv_key(priv_key: str):
from decimal import Decimal
from typing import Dict, List, Optional

from human_protocol_sdk.constants import NETWORKS, ChainId, Status
from human_protocol_sdk.constants import (
ESCROW_BULK_PAYOUT_MAX_ITEMS,
NETWORKS,
ChainId,
Status,
)
from human_protocol_sdk.utils import (
get_escrow_interface,
get_factory_interface,
Expand Down Expand Up @@ -780,6 +785,8 @@ def ensure_correct_bulk_payout_input(
raise EscrowClientError(f"Invalid recipient address: {recipient}")
if len(recipients) == 0:
raise EscrowClientError("Arrays must have any value")
if len(recipients) > ESCROW_BULK_PAYOUT_MAX_ITEMS:
raise EscrowClientError("Too many recipients")
if len(recipients) != len(amounts):
raise EscrowClientError("Arrays must have same length")
if 0 in amounts:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,25 @@ def test_bulk_payout_empty_arrays(self):
)
self.assertEqual("Arrays must have any value", str(cm.exception))

def test_bulk_payout_exceed_max_count(self):
escrow_address = "0x1234567890123456789012345678901234567890"
recipients = ["0x1234567890123456789012345678901234567890"] * 100
amounts = [100] * 100
final_results_url = "https://www.example.com/result"
final_results_hash = "test"
txId = 1

with self.assertRaises(EscrowClientError) as cm:
self.escrow.bulk_payout(
escrow_address,
recipients,
amounts,
final_results_url,
final_results_hash,
txId,
)
self.assertEqual("Too many recipients", str(cm.exception))

def test_bulk_payout_zero_amount(self):
escrow_address = "0x1234567890123456789012345678901234567890"
recipients = ["0x1234567890123456789012345678901234567890"]
Expand Down Expand Up @@ -1124,34 +1143,6 @@ def test_bulk_payout_invalid_escrow_address(self):
"Escrow address is not provided by the factory", str(cm.exception)
)

def test_bulk_payout_exceed_max_count(self):
mock_contract = MagicMock()
mock_contract.functions.bulkPayOut = MagicMock()
mock_contract.functions.bulkPayOut.return_value.transact.side_effect = Exception(
"Error: VM Exception while processing transaction: reverted with reason string 'Too many recipients'."
)
self.escrow._get_escrow_contract = MagicMock(return_value=mock_contract)
self.escrow.get_balance = MagicMock(return_value=100)
escrow_address = "0x1234567890123456789012345678901234567890"
recipients = ["0x1234567890123456789012345678901234567890"]
amounts = [100]
final_results_url = "https://www.example.com/result"
final_results_hash = "test"
txId = 1

with self.assertRaises(EscrowClientError) as cm:
self.escrow.bulk_payout(
escrow_address,
recipients,
amounts,
final_results_url,
final_results_hash,
txId,
)
self.assertEqual(
"Bulk Payout transaction failed: Too many recipients", str(cm.exception)
)

def test_bulk_payout_exceed_max_value(self):
mock_contract = MagicMock()
mock_contract.functions.bulkPayOut = MagicMock()
Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/typescript/human-protocol-sdk/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,5 @@ export const Role = {
};

export const SUBGRAPH_API_KEY_PLACEHOLDER = '[SUBGRAPH_API_KEY]';

export const ESCROW_BULK_PAYOUT_MAX_ITEMS = 99;
5 changes: 5 additions & 0 deletions packages/sdk/typescript/human-protocol-sdk/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ export const ErrorRecipientCannotBeEmptyArray = new Error(
'Recipient cannot be an empty array'
);

/**
* @constant {Error} - Too many recipients.
*/
export const ErrorTooManyRecipients = new Error('Too many recipients');

/**
* @constant {Error} - Amount must be greater than zero..
*/
Expand Down
11 changes: 10 additions & 1 deletion packages/sdk/typescript/human-protocol-sdk/src/escrow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import {
import { ContractRunner, EventLog, Overrides, Signer, ethers } from 'ethers';
import gqlFetch from 'graphql-request';
import { BaseEthersClient } from './base';
import { DEFAULT_TX_ID, NETWORKS } from './constants';
import {
DEFAULT_TX_ID,
ESCROW_BULK_PAYOUT_MAX_ITEMS,
NETWORKS,
} from './constants';
import { requiresSigner } from './decorators';
import { ChainId, OrderDirection } from './enums';
import {
Expand All @@ -33,6 +37,7 @@ import {
ErrorProviderDoesNotExist,
ErrorRecipientAndAmountsMustBeSameLength,
ErrorRecipientCannotBeEmptyArray,
ErrorTooManyRecipients,
ErrorTotalFeeMustBeLessThanHundred,
ErrorTransferEventNotFoundInTransactionLogs,
ErrorUnsupportedChainID,
Expand Down Expand Up @@ -1002,6 +1007,10 @@ export class EscrowClient extends BaseEthersClient {
throw ErrorRecipientCannotBeEmptyArray;
}

if (recipients.length > ESCROW_BULK_PAYOUT_MAX_ITEMS) {
throw ErrorTooManyRecipients;
}

if (amounts.length === 0) {
throw ErrorAmountsCannotBeEmptyArray;
}
Expand Down
41 changes: 41 additions & 0 deletions packages/sdk/typescript/human-protocol-sdk/test/escrow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
ErrorRecipientAndAmountsMustBeSameLength,
ErrorRecipientCannotBeEmptyArray,
ErrorSigner,
ErrorTooManyRecipients,
ErrorTotalFeeMustBeLessThanHundred,
ErrorUnsupportedChainID,
ErrorUrlIsEmptyString,
Expand Down Expand Up @@ -943,6 +944,26 @@ describe('EscrowClient', () => {
).rejects.toThrow(ErrorRecipientCannotBeEmptyArray);
});

test('should throw an error if too many recipients', async () => {
const escrowAddress = ethers.ZeroAddress;
const recipients = Array.from({ length: 100 }, () => ethers.ZeroAddress);
const amounts = recipients.map(() => 100n);
const finalResultsUrl = VALID_URL;
const finalResultsHash = FAKE_HASH;

escrowClient.escrowFactoryContract.hasEscrow.mockReturnValue(true);

await expect(
escrowClient.bulkPayOut(
escrowAddress,
recipients,
amounts,
finalResultsUrl,
finalResultsHash
)
).rejects.toThrow(ErrorTooManyRecipients);
});

test('should throw an error if amounts length is equal to 0', async () => {
const escrowAddress = ethers.ZeroAddress;
const recipients = [ethers.ZeroAddress];
Expand Down Expand Up @@ -1267,6 +1288,26 @@ describe('EscrowClient', () => {
).rejects.toThrow(ErrorRecipientCannotBeEmptyArray);
});

test('should throw an error if too many recipients', async () => {
const escrowAddress = ethers.ZeroAddress;
const recipients = Array.from({ length: 100 }, () => ethers.ZeroAddress);
const amounts = recipients.map(() => 100n);
const finalResultsUrl = VALID_URL;
const finalResultsHash = FAKE_HASH;

escrowClient.escrowFactoryContract.hasEscrow.mockReturnValue(true);

await expect(
escrowClient.createBulkPayoutTransaction(
escrowAddress,
recipients,
amounts,
finalResultsUrl,
finalResultsHash
)
).rejects.toThrow(ErrorTooManyRecipients);
});

test('should throw an error if amounts length is equal to 0', async () => {
const escrowAddress = ethers.ZeroAddress;
const recipients = [ethers.ZeroAddress];
Expand Down

0 comments on commit 90708c3

Please sign in to comment.