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: get TTKN appears duplicated transaction #266

Merged
merged 2 commits into from
Feb 14, 2023
Merged

fix: get TTKN appears duplicated transaction #266

merged 2 commits into from
Feb 14, 2023

Conversation

oriole-pub
Copy link
Collaborator

Discribe

Currently, there is no better solution to avoid double spend when constructing transactions, so use the simplest error prompt for processing

Related problem

@vercel
Copy link

vercel bot commented Feb 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
godwoken-bridge-mainnet ✅ Ready (Inspect) Visit Preview Feb 13, 2023 at 1:19PM (UTC)
godwoken-bridge-testnet ✅ Ready (Inspect) Visit Preview Feb 13, 2023 at 1:19PM (UTC)

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop@4ab483e). Click here to learn what that means.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #266   +/-   ##
==========================================
  Coverage           ?   65.26%           
==========================================
  Files              ?       39           
  Lines              ?     6161           
  Branches           ?      236           
==========================================
  Hits               ?     4021           
  Misses             ?     2137           
  Partials           ?        3           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ab483e...0b3dc79. Read the comment docs.

@ShookLyngs
Copy link
Collaborator

ShookLyngs commented Feb 14, 2023

I didn't see the conversation last day in #197, sorry about that.

This issue is not detailed enough, but I remember it was about state management. In the current version, a user can click the Get 1,000 Test Token(TTKN) on L1 button on the popup twice and trigger a double spend, @Dawn-githup was this the issue?

If it was, for the click twice issue, I think maybe we can add a state control logic to the button, that when a faucet method is running, the user cannot trigger the method again in the same page.

@Dawn-githup
Copy link

Dawn-githup commented Feb 14, 2023

was this the issue

yes, That's the problem

From what you've said, it's possible to add a click rate control to the button, but it would take about 5 seconds to avoid this problem.Whether this will affect the user experience, click no response

@oriole-pub
Copy link
Collaborator Author

I didn't see the conversation last day in #197, sorry about that.

This issue is not detailed enough, but I remember it was about state management. In the current version, a user can click the Get 1,000 Test Token(TTKN) on L1 button on the popup twice and trigger a double spend, @Dawn-githup was this the issue?

If it was, for the click twice issue, I think maybe we can add a state control logic to the button, that when a faucet method is running, the user cannot trigger the method again in the same page.

When the user refreshes the page, the status of the button will become invalid, and you can continue to click. Of course, this does not necessarily happen

But I think simple error prompt is an effective way to avoid introducing more complexity
@Dawn-githup @ShookLyngs

@Flouse Flouse merged commit b5f510d into godwokenrises:develop Feb 14, 2023
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.

'Get 1000 Test Token (TTKN) on L1' Data duplication error occurs after two consecutive operations
5 participants