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

fix!: fix tx and txo validation callback bug in wallet FFI #3695

Conversation

philipr-za
Copy link
Contributor

Description

A recent change to the transaction service events meant that the Transaction Validation callback was only being called when state changed and not on success like it used to.
It was also noted that the callbacks no longer reported when a validation failed.

This PR updates the Tx and TXO validation callbacks to be called on completion of the respective validation protocols and also report if it was a success or not. Due to the simplification of the validation protocols the callbacks were simplified so that the feedback is a boolean denoting success or failure.

How Has This Been Tested?

unit tests updated

A recent change to the transaction service events meant that the Transaction Validation callback was only being called when state changed and not on success like it used to.
It was also noted that the callbacks no longer reported when a validation failed.

This PR updates the Tx and TXO validation callbacks to be called on completion of the respective validation protocols and also report if it was a success or not. Due to the simplification of the validation protocols the callbacks were simplified so that the feedback is a boolean denoting success or failure.
stringhandler
stringhandler previously approved these changes Jan 11, 2022
SWvheerden
SWvheerden previously approved these changes Jan 12, 2022
@delta1
Copy link
Contributor

delta1 commented Jan 12, 2022

Is that FFI integration test flaky? Maybe try rerun it

@philipr-za
Copy link
Contributor Author

Is that FFI integration test flaky? Maybe try rerun it

I think its an issue in the Weatherwax branch. I realized I should retarget this PR to the dev branch anyway

@philipr-za philipr-za changed the base branch from weatherwax to development January 12, 2022 10:52
@philipr-za philipr-za dismissed stale reviews from SWvheerden and stringhandler January 12, 2022 10:52

The base branch was changed.

@aviator-app aviator-app bot merged commit 16cfa22 into tari-project:development Jan 14, 2022
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.

4 participants