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

Support "amend a pending transaction" #3045

Merged
merged 16 commits into from Mar 14, 2024
Merged

Support "amend a pending transaction" #3045

merged 16 commits into from Mar 14, 2024

Conversation

devchenyan
Copy link
Collaborator

@devchenyan devchenyan commented Feb 26, 2024

issue: Magickbase/neuron-public-issues#332

Screen-2024-02-27-160430.mp4

@Danie0918
Copy link
Collaborator

@yanguoyu @homura Please have a review.

Copy link
Collaborator

@homura homura left a comment

Choose a reason for hiding this comment

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

I'm not sure if the AmendTransaction entity is necessary for this PR. The amendHash appears to be the same as the original transaction's hash on their feature, which makes me wonder if we can remove AmendTransaction.

To implement the amends transaction feature in Neuron, we would need to:

  1. Add an "amend" button for each pending transaction.
  2. Create an "amend" page that allows users to increase the transaction fee.
  3. Fetch the minimal RBF fee and use it as the default amend fee.
  4. Send the amended transaction.

In my opinion, the AmendTransaction is unnecessary for this feature and could be removed.

packages/neuron-ui/src/components/AmendSend/hooks.ts Outdated Show resolved Hide resolved
@Danie0918
Copy link
Collaborator

@homura @yanguoyu Please have a review.

@Danie0918
Copy link
Collaborator

@homura Please have a review.

@silySuper
Copy link
Collaborator

/package

@silySuper
Copy link
Collaborator

I can not see amend feature in this package for mac-arm 64:https://github.com/nervosnetwork/neuron/actions/runs/8169785200?pr=3045
截屏2024-03-07 15 04 55

@devchenyan
Copy link
Collaborator Author

/package

@silySuper
Copy link
Collaborator

1.After amend twice(0.00000601 CKB-->0.00001502 CKB-->0.00002403 CKB ),2403 fee transaction will failed,lower fee will disappear.After a while 1502 fee transaction status shows confirming.

2024-03-11.15.45.02.mov

2.Send to address can not see whole in amend page.

截屏2024-03-11 15 28 48

3.Failed status can not see in history sometimes,but can see somtimes.Also as amend feature shows when transaction is pending status (data in 2024-03-11 is amend transation),Why?

截屏2024-03-11 15 53 46

@devchenyan

@Danie0918
Copy link
Collaborator

3.Failed status can not see in history sometimes,but can see somtimes.Also as amend feature shows when transaction is pending status (data in 2024-03-11 is amend transation),Why?

This feature is intended to solve the problem of pending transactions not being able to be packaged and therefore provides modification operations only in the pending state.

As for the failure state not being visible, it may be related to the requirement. #Magickbase/neuron-public-issues#332 (comment)

@silySuper
Copy link
Collaborator

4.sudt transaction amend will throw error.
截屏2024-03-11 16 26 20

@silySuper
Copy link
Collaborator

silySuper commented Mar 11, 2024

5.sudt amend page is not correct.In expect it should like sudt send page ,but actually is like this:

截屏2024-03-11 16 45 33

sudt page:

截屏2024-03-11 17 20 01

@silySuper
Copy link
Collaborator

6.ckb asset account receive lost amend feature.

2024-03-11.17.09.27.mov

@devchenyan
Copy link
Collaborator Author

devchenyan commented Mar 11, 2024

@silySuper
Copy link
Collaborator

silySuper commented Mar 12, 2024

All is same as before ,please check whether it is the testing package

@devchenyan
Copy link
Collaborator Author

devchenyan commented Mar 12, 2024

@silySuper
Copy link
Collaborator

/package

@yanguoyu
Copy link
Collaborator

yanguoyu commented Mar 12, 2024

/package
Packaging for test is done in 8244834788. @yanguoyu

@silySuper
Copy link
Collaborator

1.verified
2.verified
3.verified
4.same as before
截屏2024-03-12 15 10 43
5.verified
6.not need to change

@silySuper
Copy link
Collaborator

7.amend page verify custom price will cause automatic return to home page.

2024-03-12.15.14.40.mov

@devchenyan
Copy link
Collaborator Author

devchenyan commented Mar 12, 2024

/package
Packaging for test is done in 8253604787. @devchenyan

@silySuper
Copy link
Collaborator

4.verified
7.verified

@silySuper
Copy link
Collaborator

8.Amend transaction send pageshows abnormal when transaction sent by consuming cell(cell management)

image

9.In Nervos Dao ,amend trasaction type is changed to sent,in expect it should shows Nervos Dao
截屏2024-03-13 15 25 54

@devchenyan
Copy link
Collaborator Author

devchenyan commented Mar 13, 2024

/package
Packaging for test is done in 8261934198. @devchenyan

@silySuper
Copy link
Collaborator

8.verified
9.verified

@Keith-CY Keith-CY merged commit 57b018e into develop Mar 14, 2024
23 checks passed
@Keith-CY Keith-CY deleted the fix-332 branch March 14, 2024 06:25
yanguoyu pushed a commit to yanguoyu/neuron that referenced this pull request Mar 14, 2024
Co-authored-by: Chen Yu <keithwhisper@gmail.com>
@Keith-CY Keith-CY mentioned this pull request Mar 14, 2024
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.

None yet

6 participants