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 shutdown inputs error #53

Closed
wants to merge 1 commit into from

Conversation

chenyukang
Copy link
Collaborator

@chenyukang chenyukang commented Jun 9, 2024

Seems we mixed commitment signed tx and shutdown tx here, it will make shutdown transaction use the same inputs from funding tx, since funding tx is committed, submit shutdown tx will get an error:

TransactionFailedToResolve: Resolve failed Unknown

@contrun
Copy link
Collaborator

contrun commented Jun 10, 2024

Can you elaborate on the error encountered? Please give all the context of the transaction whose submission raises error TransactionFailedToResolve: Resolve failed Unknown (e.g. what the raw transaction data structure it is, what is the status of all the dependent cells). My intention for the function aggregate_partial_signatures_to_consume_funding_cell was always to create a valid witness to consume the funding cell (i.e. not to consume the inputs of funding transaction, but the funding transaction outputs). It seems to me the callers of aggregate_partial_signatures_to_consume_funding_cell, sign_tx_to_consume_funding_cell and another code snippet you modified are indeed consuming the funding transaction output (funding cell). I made the implicit assumption that the tx passed to aggregate_partial_signatures_to_consume_funding_cell has already used funding cell as its input. Please give raw transaction data structure to verify this. If this is not the case, we may need to modify the constructed tx, or change implementation of aggregate_partial_signatures_to_consume_funding_cell, and rename it to something like fill_in_inputs_and_witnesses_for_tx_to_consume_funding_cell (This name is bad. My point is to always modify the transaction inputs and witnesses instead of passing shutdown parameter. Because this is redundant, I always assume the input is funding cell).

@quake
Copy link
Member

quake commented Jun 10, 2024

Can you elaborate on the error encountered? Please give all the context of the transaction whose submission raises error TransactionFailedToResolve: Resolve failed Unknown (e.g. what the raw transaction data structure it is, what is the status of all the dependent cells). My intention for the function aggregate_partial_signatures_to_consume_funding_cell was always to create a valid witness to consume the funding cell (i.e. not to consume the inputs of funding transaction, but the funding transaction outputs). It seems to me the callers of aggregate_partial_signatures_to_consume_funding_cell, sign_tx_to_consume_funding_cell and another code snippet you modified are indeed consuming the funding transaction output (funding cell). I made the implicit assumption that the tx passed to aggregate_partial_signatures_to_consume_funding_cell has already used funding cell as its input. Please give raw transaction data structure to verify this. If this is not the case, we may need to modify the constructed tx, or change implementation of aggregate_partial_signatures_to_consume_funding_cell, and rename it to something like fill_in_inputs_and_witnesses_for_tx_to_consume_funding_cell (This name is bad. My point is to always modify the transaction inputs and witnesses instead of passing shutdown parameter. Because this is redundant, I always assume the input is funding cell).

this error may be triggered by e2e test, ref: https://github.com/contrun/ckb-pcn-node/actions/runs/9434095793/job/25985824223#step:6:1769

[2024-06-09T05:02:06Z ERROR ckb_pcn_node::ckb_chain::actor] [ckb-chain] send transaction failed: Rpc(Error { code: ServerError(-301), message: "TransactionFailedToResolve: Resolve failed Unknown(OutPoint(0x1bbdf535281ade12428e33e7a5839d1c0b52da66f5cf853c01f703874447826700000000))", data: Some(String("Resolve(Unknown(OutPoint(0x1bbdf535281ade12428e33e7a5839d1c0b52da66f5cf853c01f703874447826700000000)))")) }) [node 3]
[2024-06-09T05:02:06Z ERROR ckb_pcn_node::ckb_chain::actor] [ckb-chain] send transaction failed: Rpc(Error { code: ServerError(-301), message: "TransactionFailedToResolve: Resolve failed Unknown(OutPoint(0x1bbdf535281ade12428e33e7a5839d1c0b52da66f5cf853c01f703874447826700000000))", data: Some(String("Resolve(Unknown(OutPoint(0x1bbdf535281ade12428e33e7a5839d1c0b52da66f5cf853c01f703874447826700000000)))")) }) [node 1]

@contrun
Copy link
Collaborator

contrun commented Jun 10, 2024

There was an overlook in #39 in which I copied some of the old code to build a transaction to consume the funding cell (including commitment transactions and shutdown transaction). This created an wrong transaction. I fixed this error in #57.

It seems that the error in building commitment transaction was not caught earlier because the transaction verifier of ckb-testtool only considers whether the scripts in the transaction can run correctly and it is OK for a transaction to have already consumed inputs. Moreover, the error in e2e test was not correctly captured in CI (only showing the error in log).

This PR is closed in favor of #57

@contrun contrun closed this Jun 10, 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.

3 participants