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.

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

Add Payment Hash to response and better error messages #51

Merged
merged 5 commits into from
Jan 22, 2024
+70 −12
Merged

Add Payment Hash to response and better error messages #51

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8836a31
add payment Hash
jonathanlei Dec 19, 2023
5eef476
revert bigQuery import
jonathanlei Dec 19, 2023
11d9c8f
fix nits and add more logging
jonathanlei Dec 21, 2023
9fa56fd
add env file, fix error messages
jonathanlei Jan 11, 2024
135137b
add ticket error code logging
jonathanlei Jan 11, 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
30 changes: 24 additions & 6 deletions src/routes/accounts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,20 @@ export default async function (req: Request, res: Response) {

try {
let result;
let paymentHash;
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the signing out of the the try. That way you dont have to create the variable paymentHash.

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 moved it out, the purpose of having it there is that the hash should be different before and after submitting right? I could have an error statement catching signing errors too.

Copy link
Contributor

Choose a reason for hiding this comment

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

The hash will be the same before and after

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then we don't need signing

Copy link
Contributor

Choose a reason for hiding this comment

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

Signing is used to add the signature field but that doesnt change the hash as far as I know.

Copy link
Contributor

Choose a reason for hiding this comment

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

The signature is among the data used to compute a transaction's identifying hash. Adding or changing the signature will change the transaction hash. Relatedly, you can learn about transaction malleability here. Generally speaking, I don't consider any given transaction to have a hash yet if it is not signed (yet).

The hash is the same before and after submitting, if you are submitting a signed transaction.

result = await submitPaymentWithTicket(payment, client, fundingWallet);
const signedPayment = await fundingWallet.sign(payment);
paymentHash = signedPayment.hash;
let response = await submitPaymentWithTicket(
payment,
client,
fundingWallet
);
({ result, hash: paymentHash } = response);
} catch (err) {
console.log(`${rTracer.id()} | Failed to submit payment: ${err}`);
console.log(
`${rTracer.id()} | Failed to submit payment ${paymentHash}: ${err}`
);
res.status(500).send({
error: "Unable to fund account. Try again later",
account,
Expand All @@ -88,6 +98,7 @@ export default async function (req: Request, res: Response) {
const response: FundedResponse = {
account: account,
amount: Number(amount),
paymentHash: paymentHash,
};

if (wallet && wallet.seed) {
Expand All @@ -98,7 +109,7 @@ export default async function (req: Request, res: Response) {
console.log(
`${rTracer.id()} | Funded ${
account.address
} with ${amount} XRP (${status})`
} with ${amount} XRP (${status}), paymentHash: ${paymentHash}`
);

if (config.BIGQUERY_PROJECT_ID) {
Expand All @@ -109,6 +120,7 @@ export default async function (req: Request, res: Response) {
console.warn(`Failed to insert into BigQuery: ${error}`);
}
}

incrementTxCount();
res.send(response);
}
Expand All @@ -120,6 +132,7 @@ export default async function (req: Request, res: Response) {
});
}
}

async function insertIntoBigQuery(
account: Account,
amount: string,
Expand Down Expand Up @@ -173,12 +186,17 @@ async function submitPaymentWithTicket(
) {
let retryCount = 0;
let result;
let hash;
while (retryCount < maxRetries) {
payment.TicketSequence = await getTicket(client);
result = (await client.submit(payment, { wallet: fundingWallet })).result;
payment = await client.autofill(payment);
const { tx_blob: paymentBlob, hash: paymentHash } =
await fundingWallet.sign(payment);
hash = paymentHash;
result = (await client.submit(paymentBlob)).result;
if (result.engine_result === "tefNO_TICKET") {
retryCount++;
console.log(`Retrying transaction (${retryCount}/${maxRetries})`);
console.log(`Retrying transaction ${hash} (${retryCount}/${maxRetries})`);
} else {
break;
}
Expand All @@ -188,5 +206,5 @@ async function submitPaymentWithTicket(
throw new Error("Failed to submit transaction after multiple attempts");
}

return result;
return { result, hash };
}
1 change: 1 addition & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ export interface Account {
export interface FundedResponse {
account: Account;
amount: number;
paymentHash: string;
seed?: string;
}