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

RED-201 Network Generated Transactions #65

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

ahmxdiqbal
Copy link
Member

PR for archiver changes of the reliable reward txs aka network generated txs project

@ahmxdiqbal ahmxdiqbal changed the title Service queue impl RED-201 Service queue impl Aug 21, 2024
Copy link

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

Error Handling
The new endpoint '/network-txs-list' lacks error handling. If ServiceQueue.getTxList() fails or encounters an issue, the server will still attempt to send a response which might not be meaningful. Consider adding error handling to manage such cases gracefully.

Error Handling
The function updateNetworkTxsList logs errors but does not handle them in a way that affects the flow of the application. This could lead to inconsistent states if syncTxList fails. Consider handling these errors more robustly or rethrowing them to be handled at a higher level.

Potential Bug
The function addTxs and removeTxs in ServiceQueue.ts return a boolean indicating success or failure, but these return values are not checked by any caller. This might lead to unnoticed failures in transaction handling.

@ahmxdiqbal ahmxdiqbal changed the title RED-201 Service queue impl RED-201 Network Generated Transactions Aug 21, 2024
export function addTxs(addTxs: P2P.ServiceQueueTypes.AddNetworkTx[]): boolean {
try {
for (const addTx of addTxs) {
Logger.mainLogger.info(`Adding network tx of type ${addTx.type} and payload ${stringifyReduce(addTx.txData)}`)

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.
for (const removeTx of removeTxs) {
const index = txList.findIndex((entry) => entry.hash === removeTx.txHash)
if (index === -1) {
Logger.mainLogger.error(`TxHash ${removeTx.txHash} does not exist in txList`)

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.
write updateNetworkTxsList func

create /network-txs-list endpoint

fix: improper logic in case txRemove tx is not in txList

ServiceQueue: updated sortedInsert() definition and calls

ServiceQueue: fix Logger import

add verify function for syncing txList

add queries for syncing txList from validators

add syncTxList func + use it in syncV2 step

add ServiceQueue funcs + minor refactor

handle case where txList isnt correct post-cycle record parse
@mhanson-github mhanson-github merged commit 41b5c1f into dev Sep 11, 2024
6 of 7 checks passed
@mhanson-github mhanson-github deleted the serviceQueue-impl branch September 23, 2024 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants