Skip to content

refactor: we should expect one output for each data instead of an array of data outputs #34

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

Merged
merged 26 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f162520
feat: added sendtransaction method and types
andreabadesso Feb 12, 2025
0b1a2e5
feat: send transaction rpc method, validation and lib update
andreabadesso Feb 24, 2025
4eacc1e
refactor: removed unused console.log
andreabadesso Feb 24, 2025
4583529
tests: added tests for sendtransaction
andreabadesso Feb 24, 2025
b2f9b5f
refactor: removed invalid cast
andreabadesso Feb 24, 2025
93cb6b5
refactor: revert to correct version and package name
andreabadesso Feb 24, 2025
9362675
docs: better comments on pin mock
andreabadesso Mar 6, 2025
cd77135
chore: removed unused import
andreabadesso Mar 6, 2025
1d56a1b
chore: added missing deps
andreabadesso Mar 6, 2025
8d7166c
chore: removed long and added zod
andreabadesso Mar 6, 2025
0012362
refactor: we should not add 0.01 htr automatically when data output
andreabadesso Mar 7, 2025
b5879f0
refactor: revert to manually adding 1 HTR to data outputs
andreabadesso Mar 7, 2025
30a775e
feat: sending specific error for failure in prepare tx
andreabadesso Mar 13, 2025
c2df73d
fix: create token rpc should accept strings and output bigint
andreabadesso Mar 14, 2025
a86e8c0
feat: transforming amount in sendNanoContractTx to bigint
andreabadesso Mar 17, 2025
d9fe22f
tests: added tests for bigint transformations
andreabadesso Mar 17, 2025
a408025
Merge branch 'master' into feat/bigint-support
andreabadesso Mar 17, 2025
8e274f4
refactor: using coerse to transform to bigint
andreabadesso Mar 17, 2025
154880f
refactor: we should reject zero values for send nano contract tx amount
andreabadesso Mar 20, 2025
ac11418
refactor: sendTransaction should expect only positive values for outputs
andreabadesso Mar 20, 2025
791fd97
refactor: custom zod schema for NanoContractActionSchema
andreabadesso Mar 26, 2025
2a8db7a
refactor: BigInt -> n
andreabadesso Mar 27, 2025
972f504
feat: splitting data outputs into multiple outputs
andreabadesso Mar 20, 2025
8822505
test: add tests for data output splitting functionality
andreabadesso Mar 20, 2025
76d2f94
refactor: stop accepting an string array as data
andreabadesso Mar 20, 2025
110e593
Merge branch 'master' into feat/one-output-per-data
andreabadesso Apr 1, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('sendNanoContractTx', () => {
const expectedActions = [
{
...(rpcRequest.params.actions[0] as unknown as NanoContractActionWithStringAmount),
amount: BigInt(100), // Expected conversion to BigInt
amount: 100n, // Expected conversion to BigInt
}
];

Expand Down Expand Up @@ -147,11 +147,11 @@ describe('sendNanoContractTx', () => {
const expectedActions = [
{
...stringActions[0],
amount: BigInt(100),
amount: 100n,
},
{
...stringActions[1],
amount: BigInt(200),
amount: 200n,
},
];

Expand Down Expand Up @@ -268,7 +268,7 @@ describe('sendNanoContractTx', () => {
args: rpcRequest.params.args,
actions: [{
...originalAction,
amount: BigInt(100), // Convert amount to BigInt
amount: 100n, // Convert amount to BigInt
}],
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,15 @@ describe('sendTransaction', () => {
inputs: [{
txId: 'testTxId',
index: 0,
value: 100,
value: 100n,
address: 'testAddress',
token: '00',
}],
outputs: [{
address: 'testAddress',
value: BigInt(100),
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't all values be bigint? Should we change the input value as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we should, done

Copy link
Member

Choose a reason for hiding this comment

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

Why not 100n?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored everywhere to use n instead of BigInt

token: '00',
}],
}),
run: sendTransactionMock,
}),
Expand Down Expand Up @@ -104,13 +109,13 @@ describe('sendTransaction', () => {
data: {
outputs: [{
address: 'testAddress',
value: BigInt(100),
value: 100n,
token: '00',
}],
inputs: [{
txId: 'testTxId',
index: 0,
value: 100,
value: 100n,
address: 'testAddress',
token: '00',
}],
Expand All @@ -127,7 +132,7 @@ describe('sendTransaction', () => {
rpcRequest.params.outputs = [{
type: 'data',
value: '100',
data: ['test data'],
data: 'test data',
}];

promptHandler
Expand All @@ -147,15 +152,75 @@ describe('sendTransaction', () => {
// Verify data output was transformed correctly
expect(wallet.sendManyOutputsSendTransaction).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
type: 'data',
value: 1n,
token: '00',
data: 'test data',
}),
]),
expect.any(Object),
);
});

it('should handle mix of data and regular outputs correctly', async () => {
rpcRequest.params.outputs = [
{
address: 'testAddress1',
value: '100',
token: '00',
},
{
type: 'data',
data: 'data item',
value: '1',
},
{
address: 'testAddress2',
value: '200',
token: '00',
}
];

promptHandler
.mockResolvedValueOnce({
type: TriggerResponseTypes.SendTransactionConfirmationResponse,
data: { accepted: true },
})
.mockResolvedValueOnce({
type: TriggerResponseTypes.PinRequestResponse,
data: { accepted: true, pinCode: '1234' },
});

sendTransactionMock.mockResolvedValue({ hash: 'txHash123' });

await sendTransaction(rpcRequest, wallet, {}, promptHandler);

// Verify the transformation preserves regular outputs and handles data output
expect(wallet.sendManyOutputsSendTransaction).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
address: 'testAddress1',
value: BigInt(100),
token: '00',
}),
expect.objectContaining({
type: 'data',
value: BigInt(1),
token: '00',
data: ['test data'],
data: 'data item',
}),
expect.objectContaining({
address: 'testAddress2',
value: BigInt(200),
token: '00',
}),
]),
expect.any(Object),
);

// Verify the array length matches the expected number of outputs (2 regular + 1 data output)
expect(wallet.sendManyOutputsSendTransaction.mock.calls[0][0]).toHaveLength(3);
});

it('should throw InvalidParamsError for invalid request parameters', async () => {
Expand Down
35 changes: 20 additions & 15 deletions packages/hathor-rpc-handler/src/rpcMethods/sendTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const sendTransactionSchema = z.object({
.optional(),
token: z.string().optional(),
type: z.string().optional(),
data: z.array(z.string()).optional(),
data: z.string().optional(),
})
.transform(output => {
// If data is present, automatically set type to 'data'
Expand All @@ -58,7 +58,7 @@ const sendTransactionSchema = z.object({
}
return true;
}, {
message: "Value is required when data is not provided"
message: 'Value is required when data is not provided'
})).min(1),
inputs: z.array(z.object({
txId: z.string(),
Expand Down Expand Up @@ -100,16 +100,27 @@ export async function sendTransaction(
validateNetwork(wallet, params.network);

// Transform outputs before sending to wallet-lib
const transformedOutputs = params.outputs.map(output => {
const transformedOutputs = params.outputs.reduce<Array<{
address?: string;
value?: bigint;
token?: string;
type?: string;
data?: string;
}>>((acc, output) => {
const result = { ...output };

if (result.type === 'data' || (result.data && result.data.length > 0)) {
result.value = BigInt(1);
result.token = constants.NATIVE_TOKEN_UID;
// We should manually set the 0.01 HTR to data output
// so it's displayed to the user during confirmation.
if (result.type === 'data') {
return [...acc, {
...output,
value: 1n,
token: constants.NATIVE_TOKEN_UID,
}];
}

return result;
});
return [...acc, result];
}, []);

// sendManyOutputsSendTransaction throws if it doesn't receive a pin,
// but doesn't use it until prepareTxData is called, so we can just assign
Expand Down Expand Up @@ -142,13 +153,7 @@ export async function sendTransaction(
type: TriggerTypes.SendTransactionConfirmationPrompt,
method: rpcRequest.method,
data: {
outputs: params.outputs as unknown as Array<{
address?: string;
value: number;
token?: string;
type?: string;
data?: string[];
}>,
outputs: preparedTx.outputs,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was a bug, we were always sending params.outputs, so we change output would never reach our client to review

inputs: preparedTx.inputs,
changeAddress: params.changeAddress,
}
Expand Down
2 changes: 1 addition & 1 deletion packages/hathor-rpc-handler/src/types/rpcRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export interface SendTransactionRpcRequest {
value: string | number | bigint;
token?: string;
type?: string;
data?: string[];
data?: string;
}>;
inputs?: Array<{
txId: string;
Expand Down