-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from all commits
f162520
0b1a2e5
4eacc1e
4583529
b2f9b5f
93cb6b5
9362675
cd77135
1d56a1b
8d7166c
0012362
b5879f0
30a775e
c2df73d
a86e8c0
d9fe22f
a408025
8e274f4
154880f
ac11418
791fd97
2a8db7a
972f504
8822505
76d2f94
110e593
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored everywhere to use |
||
token: '00', | ||
}], | ||
}), | ||
run: sendTransactionMock, | ||
}), | ||
|
@@ -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', | ||
}], | ||
|
@@ -127,7 +132,7 @@ describe('sendTransaction', () => { | |
rpcRequest.params.outputs = [{ | ||
type: 'data', | ||
value: '100', | ||
data: ['test data'], | ||
data: 'test data', | ||
}]; | ||
|
||
promptHandler | ||
|
@@ -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 () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' | ||
|
@@ -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(), | ||
|
@@ -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 | ||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a bug, we were always sending |
||
inputs: preparedTx.inputs, | ||
changeAddress: params.changeAddress, | ||
} | ||
|
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.
Shouldn't all values be bigint? Should we change the input value as well?
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.
Yes we should, done