Skip to content

fix: Offline multisig with complex transaction - #3291

Merged
devchenyan merged 7 commits into
developfrom
feat-429
Feb 7, 2025
Merged

fix: Offline multisig with complex transaction#3291
devchenyan merged 7 commits into
developfrom
feat-429

Conversation

@devchenyan

@devchenyan devchenyan commented Jan 5, 2025

Copy link
Copy Markdown
Contributor

issue: Magickbase/neuron-public-issues#429

  • multisig
Screen-2025-01-06-004004.mp4
  • Single signatures
Screen-2025-01-26-111156.mp4

@yanguoyu

yanguoyu commented Jan 5, 2025

Copy link
Copy Markdown
Contributor

@Danie0918 Is this result expected?

On the other hand, I didn't get the design for this issue.

@Danie0918

Copy link
Copy Markdown

#3260

This popup is as expected and addresses Neuron's inability to sign transactions with unrecognized lock scripts.

@yanguoyu

yanguoyu commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

#3260

This popup is as expected and addresses Neuron's inability to sign transactions with unrecognized lock scripts.

From #3260, if we skip the unknown lock script. Neuron can sing successfully.

@Danie0918 Danie0918 changed the title fix: Offline sign with complex transaction fix: Offline multisig with complex transaction Jan 8, 2025
Comment thread packages/neuron-wallet/src/services/transaction-sender.ts
@devchenyan
devchenyan requested a review from silySuper January 11, 2025 13:18
@silySuper

silySuper commented Jan 13, 2025

Copy link
Copy Markdown

/package
Packaging for test is done in 12739913571. @silySuper

@Keith-CY

Keith-CY commented Jan 15, 2025

Copy link
Copy Markdown
Contributor

/package Packaging for test is done in 12739913571. @silySuper

How is this test going?

@silySuper

Copy link
Copy Markdown

/package Packaging for test is done in 12739913571. @silySuper

How is this test going?

Plan to finish tomorrow.

@silySuper

silySuper commented Jan 16, 2025

Copy link
Copy Markdown

I have used this json ,brocast fail and does not pop up window.
transaction_duoqian 2.json
Inputs lock is this:

截屏2025-01-16 14 47 50
2025-01-16.14.44.30.mov

main.log

@silySuper

silySuper commented Jan 16, 2025

Copy link
Copy Markdown

When click Nervos Dao,neuron crash

2025-01-16.15.58.30.mov

main.log
bundled-ckb-lignt-client-testnet.log

@silySuper

Copy link
Copy Markdown
2025-01-23.11.37.16.mov

throw error when click confirm

@devchenyan

devchenyan commented Jan 23, 2025

Copy link
Copy Markdown
Contributor Author

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

@silySuper

Copy link
Copy Markdown

Approve flow is abnormal,in expect,export transaction->sign &&export transaction ->broadcast->pop up transaction hash tip,but actual is export transaction->sign &&export transaction->sign &&export transaction(now pop up error also)->broadcast

here is actual affect video:

2025-01-23.15.43.46.mov

@devchenyan

Copy link
Copy Markdown
Contributor Author

Approve flow is abnormal,in expect,export transaction->sign &&export transaction ->broadcast->pop up transaction hash tip,but actual is export transaction->sign &&export transaction->sign &&export transaction(now pop up error also)->broadcast

here is actual affect video:

2025-01-23.15.43.46.mov

The purpose of this requirement is to skip unrecognized inputs and only sign the recognizable ones. However, it does not guarantee that the signed transaction can be successfully broadcast.

@Danie0918 please confirm.

@Danie0918

Copy link
Copy Markdown

Approve flow is abnormal,in expect,export transaction->sign &&export transaction ->broadcast->pop up transaction hash tip,but actual is export transaction->sign &&export transaction->sign &&export transaction(now pop up error also)->broadcast
here is actual affect video:
2025-01-23.15.43.46.mov

The purpose of this requirement is to skip unrecognized inputs and only sign the recognizable ones. However, it does not guarantee that the signed transaction can be successfully broadcast.

@Danie0918 please confirm.

It's okay.

@Danie0918

Copy link
Copy Markdown

Single signatures also need to support this feature.

@devchenyan

Copy link
Copy Markdown
Contributor Author

Single signatures also need to support this feature.

supported

@Danie0918
Danie0918 requested review from homura and yanguoyu January 26, 2025 02:10
@Danie0918

Copy link
Copy Markdown

@yanguoyu @homura Please have a review.

@silySuper

Copy link
Copy Markdown
截屏2025-01-26 10 41 09 ignore spell wrong

@devchenyan

Copy link
Copy Markdown
Contributor Author

截屏2025-01-26 10 41 09 ignore spell wrong

fixed
image

@silySuper

silySuper commented Jan 26, 2025

Copy link
Copy Markdown

/package
Packaging for test is done in 12972622182. @silySuper

@silySuper

Copy link
Copy Markdown

Single signatures also need to support this feature.

supported

When click confirm,it shows error.
image

This is json:
transaction_new.json

@Keith-CY

Keith-CY commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

Single signatures also need to support this feature.

supported

When click confirm,it shows error. image

This is json: transaction_new.json

@devchenyan please check this

@devchenyan

Copy link
Copy Markdown
Contributor Author

Single signatures also need to support this feature.

supported

When click confirm,it shows error. image

This is json: transaction_new.json

@devchenyan please check this

I'm working on it

@devchenyan

Copy link
Copy Markdown
Contributor Author

Approve flow is abnormal,in expect,export transaction->sign &&export transaction ->broadcast->pop up transaction hash tip,but actual is export transaction->sign &&export transaction->sign &&export transaction(now pop up error also)->broadcast
here is actual affect video:
2025-01-23.15.43.46.mov

The purpose of this requirement is to skip unrecognized inputs and only sign the recognizable ones. However, it does not guarantee that the signed transaction can be successfully broadcast.
@Danie0918 please confirm.

It's okay.

@silySuper This requirement needs to be able to skip unsigned inputs and export the transaction, so broadcasting may still fail. This situation meets the requirement.

@silySuper

Copy link
Copy Markdown

Approve flow is abnormal,in expect,export transaction->sign &&export transaction ->broadcast->pop up transaction hash tip,but actual is export transaction->sign &&export transaction->sign &&export transaction(now pop up error also)->broadcast
here is actual affect video:
2025-01-23.15.43.46.mov

The purpose of this requirement is to skip unrecognized inputs and only sign the recognizable ones. However, it does not guarantee that the signed transaction can be successfully broadcast.
@Danie0918 please confirm.

It's okay.

@silySuper This requirement needs to be able to skip unsigned inputs and export the transaction, so broadcasting may still fail. This situation meets the requirement.

OK.

@devchenyan
devchenyan added this pull request to the merge queue Feb 7, 2025
Merged via the queue into develop with commit eb7c83b Feb 7, 2025
@devchenyan
devchenyan deleted the feat-429 branch February 7, 2025 09:17
@Keith-CY Keith-CY mentioned this pull request Feb 12, 2025
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.

6 participants