Resolve state management bug

[nrutledge]

This fixes #54 with the following changes:

• Improve form validation so user is informed if name field is incorrect
• Fix logic so that state of default parent selection matches what's shown in the UI
• Add toast messages for success/failure of breeding transaction (was no feedback before)
• Disable persistence of redux store that was leading to stale data being used
• Improve feedback in UI for kitty list loading and error states (search and My Kitties pages)
• Handle error during breeding that incorrectly has 200 status in server response
• Filter out kitties with bad status in parent selection during breeding
• Fix intermittent infinite spinner / not connecting to wallet
• Minor refactoring

[NadigerAmit]

Hi @nrutledge and @AltiMario ,

I tested with the updated code from the @nrutledge

I conducted multiple tests and successfully completed breeding operations several times. Based on my findings, I can summarize the results as follows:

Summary:

1. Web Server: No issues detected.
2. Blockchain Operations: No issues detected.
3. DApp (JavaScript Code): Several issues identified.

Issues Identified:
1. Outdated Kitty Data in DApp: The JavaScript code retains old kitties and doesn't refresh when the application is started, even though the web server provides the updated list of kitties.

2. Incorrect DNA Used in Breeding: During breeding, the JavaScript code uses the DNA of an old kitty instead of the selected one. This issue was confirmed through logs from the web server, which indicated that the correct values were shown in the DApp, but incorrect values were sent during the breed operation.

Workarounds:

Issue 1: Outdated Kitty Data in DApp
Problem: The DApp displays old, deleted kitties in the "Search Kitty" and "My Kitty" pages.
Workaround:

1. In the linux terminal, navigate to "/tmp/tuxedo-wallet".
2. Run the command: "rm -rf wallet_database" to remove the old kitties from the web server's database.
3. Create new "Dad" and "Mom" kitties as outlined in the TuxedoDapp Testing Guide.
4. Click on "Search Kitty" to view the updated listing the DApp , which will now only include the newly created kitties.
   Note: Creating new kitties is necessary to refresh the list in the current DApp, as it doesn't refresh the existing list.

Issue 2: Incorrect DNA Used in Breeding
Problem: The DApp caches old kitty DNA, causing errors during breeding. An alert may display "Error breeding kitty: Cannot read properties of null," indicating that the DApp is sending outdated DNA to the web server, which then rejects the operation because the DNA doesn't match any entries in the database.
Workaround:

1. Go to "Search Kitties." in DApp
2. Select the "Mom" kitty. i.e click on that which will lead to kitty detail page.
3. Click on the "Breed" button at the bottom
4. Attempt to select the "Dad" kitty from the dropdown.
5. Enter the name of the child kitty.
6. Click on the "Breed." button
7. If you receive the error "Error breeding kitty: Cannot read properties of null," proceed to the next step.
8. Return to "Search Kitty."
9. This time, select the "Dad" kitty.
10. Then select the "Mom" kitty from the dropdown.
11. Enter the name of the child kitty.
12. Press "Breed." button This time, the Talisman wallet should appear for signing the transaction.

I believe the above steps will resolve the issues related to the DApp caching old kitty details.
Here basically JS will send the correct dnas of mom and DAD kitties to web server which are selected for breeding.

Below are some of the screen shots which were taken in during this testing :

Error due to wrong DNA

Success full breeding Case 1

After Breeding , kew child kitty shows in the "Search kitty":

Success full breeding Case 2

Make sure Talisman wallet is connected always:

[NadigerAmit]

@nrutledge and @AltiMario
If you find issues in testing, I can be available for assistance. Please book a meeting with me. I can show you the demo.

[nrutledge]

@NadigerAmit thank you for investigating and providing additional details. I've made additional changes based on that and have been able to successfully breed kitties on my end.

That being said, I was not able to fully reproduce the first issue (outdated kitty data in DApp) that you mentioned. The parent DNA was indeed stale due to unnecessary persistence of what should be transient form data. However, the kitty list was being reloaded and replaced with the response from the server after each load/refresh (despite it also being persisted). The only case where this was not true, was if the server responded with an error / no results.

Anyway, none of this should be an issue anymore with the latest changes.

[NadigerAmit]

Hi @nrutledge
CC: @AltiMario
Thank you for resolving the issues. I tested the updated DApp code, and I'm happy to report that both previously mentioned issues have been addressed. This is a big relief and has made testing much easier.

However, I noticed one minor issue:

When attempting to breed using a Mom Kitty with the status "HadBirthRecently" and a Dad Kitty with the status "Tired," the transaction status incorrectly shows "Kitty Breed successfully." While the transaction is indeed sent, it is correctly rejected by the blockchain due to the kitties' statuses, and no new child kitty is created. This behavior is as expected, but the messaging could be clearer.

Since the blockchain sends the failed transaction status asynchronously to the web server, which then relays it to the DApp, we have a few options to improve this:

Options to Address the Issue:
Option 1: Simple Fix

Update the message from "Kitty Breed successfully" to "Kitty Breed transaction sent successfully."
This provides a more accurate description of the transaction status.

Option 2: Moderate Complexity

• Update the message to "Kitty Breed transaction sent successfully."
• Additionally, listen for the REST API return value. If the transaction fails, display "Transaction failed." If successful, display "Kitty Breed successfully."

Option 3: Moderate Complexity

• Have the DApp validate the kitties' statuses before calling the REST API. Specifically, check that the Mom Kitty's status is not "HadBirthRecently" and the Dad Kitty's status is not "Tired." If either condition fails, do not call the REST API and display an appropriate message to the user.

Note: This last option may not be ideal, as there could be many invalid transactions, and having the DApp check all conditions could be resource-intensive and prone to errors due to stale data.

The above are just suggestions, There can be other better options also, Please feel free to take your decision.

PFA some of the screen shots taken when issues occoure '

1. When invlid transactions are sent from the Dapp , it shows "Kitty Breed successfully"

2. Blockchain error due to invalid txn '

3. Weservioce error, since blockchain returned error :

[JoshOrndorff]

Custom error: 0

I didn't follow the whole conversation, but I wanted to let you know that if you use a newer version of Tuxedo, you will get a more specific a descriptive error. I improved this in Off-Narrative-Labs/Tuxedo#214

[nrutledge]

@NadigerAmit @AltiMario I was able to reproduce the breeding issue with the bad parent status and have pushed some additional changes.

My initial observations from trying to reproduce the issue:

1. The web server is responding back with a 200 status despite the breed transaction failing and the response body containing an error message. This is somewhat unexpected behavior (the frontend would have displayed the error if a non success status were returned).
2. Any time a transaction fails (such as the parents having an incorrect status), it seems that all subsequent transactions fail (even using valid transactions from the testing doc with a unique child name). I had to clear the webservice db and re-initialize everything each time this happened.
3. I started experiencing an unrelated issue that was slowing down testing: the wallet was not connecting intermittently and there would be an infinite spinner at the top.

In response to the above observations, I have made the following changes:

1. Check breed response from server to ensure it contains expected success data. If missing, throw an error (which shows a toast message to the user).
2. Filter out parents in breeding selection that do not have the "RearinToGo" status (and show a message when there is no mom/dad ready for breeding). This seemed like the best approach to me, even if it may suffer from problems with stale data in some cases. The majority of the time, this should prevent invalid transactions from reaching the server and avoid frustration for the user (better to prevent incorrect behavior than to throw an error after).
3. I imposed a very small delay for the wallet connection which appears to address the connection failing intermittently.

[NadigerAmit]

@nrutledge
cc: @AltiMario
Thank you for fixing .

All the above counter measures the problem mentioned looks ok to me and it should be ok to follow the test guide without issues.

[AltiMario]

Hi @NadigerAmit and @nrutledge I did a test following the instructions from scratch and step by step. The breading issue is fixed but I cannot edit (for the scope of trading) the kitties. Here is the video to show the problem:
Edit-kitty.webm

The only errors in the log are these

My local tools:
node 22.7
rust 1.78.0
yarn 1.22.22

Am I doing something wrong?

---

After removing the wallet db (and restarting the test from scratch), the problem didn't occur anymore.