Skip to content

Bugfix/failed deposit#25

Open
hundredark wants to merge 2 commits intomainfrom
bugfix/failed-deposit
Open

Bugfix/failed deposit#25
hundredark wants to merge 2 commits intomainfrom
bugfix/failed-deposit

Conversation

@hundredark
Copy link
Contributor

No description provided.

@cedricfung
Copy link
Contributor

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f51faca357

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +56 to +57
query := fmt.Sprintf("SELECT %s FROM failed_deposits WHERE hash=?", strings.Join(failedDepositCols, ","))
row := s.db.QueryRowContext(ctx, query, hash)

Choose a reason for hiding this comment

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

P1 Badge Handle every failed deposit recorded for a transaction hash

ReadFailedDepositByHash queries failed_deposits with QueryRowContext by hash, but hash is not unique (output_id is the primary key), so a single Solana signature can have multiple failed deposit rows (different deposit outputs). In that case confirmBurnRelatedSystemCall will only recover one row and mint only one reimbursement, leaving other failed deposits for the same transaction unprocessed forever; this should load and process all unhandled rows for the hash.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants