Skip to content

Conversation

@lissavxo
Copy link
Collaborator

Related to #379

Description

Ignore Op return trigger success validation on bch transactions

Test plan

Check if button is triggering success on BCH transactions with wrong OP_RETURN

@Klakurka Klakurka requested a review from chedieck April 17, 2025 16:29
@lissavxo lissavxo force-pushed the feat/ignore_opreturn_bch branch 2 times, most recently from aa5f331 to 579a8a2 Compare May 13, 2025 19:27
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

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

Conflicts

@lissavxo lissavxo force-pushed the feat/ignore_opreturn_bch branch from 579a8a2 to f23dc8c Compare June 3, 2025 16:37
const opReturn = rawOpReturnIsEmptyOrUndefined ? message : rawOpReturn
const opReturnIsEmptyOrUndefined = opReturn === '' || opReturn === undefined;

if(!isBCH){
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is really weird.

  1. Identation is missing
  2. The first three lines of the if clause are identical to what has just been done before entering the if (!isBCH) {...}
  3. The last two lines of the if clause are identical to what will be done after leaving the if (!isBCH) {...}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should be good now

@lissavxo lissavxo requested a review from chedieck June 3, 2025 18:45
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

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

This works with no paymentId, but if paymentId is enabled then it won't trigger success without it. The paymendId, As far as I understood, being included in the opReturn, shouldn't matter.

@lissavxo lissavxo requested a review from chedieck June 4, 2025 18:35
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

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

As it is now, it works if I send the OP_RETURN without the paymentId, but not if I don't send the OP_RETURN.

As I understood, the expected behavior is: for BCH PayButtons / Widgets, completely ignore the existence of OP_RETURN.
That means that we should check if incoming transactions should be "success txs" (i.e; run onSuccess, toast, etc) solely based on the amount.

@chedieck
Copy link
Collaborator

As it is now, it works if I send the OP_RETURN without the paymentId, but not if I don't send the OP_RETURN.

As I understood, the expected behavior is: for BCH PayButtons / Widgets, completely ignore the existence of OP_RETURN. That means that we should check if incoming transactions should be "success txs" (i.e; run onSuccess, toast, etc) solely based on the amount.

Actually got it to work, must have been something else.

@chedieck chedieck self-requested a review June 19, 2025 17:12
@chedieck chedieck merged commit 9dd2447 into master Jun 19, 2025
1 check passed
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.

3 participants