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

Populating transaction witnesses signature naming convention #1745

Closed
calldelegation opened this issue Feb 12, 2024 · 1 comment · Fixed by #1815
Closed

Populating transaction witnesses signature naming convention #1745

calldelegation opened this issue Feb 12, 2024 · 1 comment · Fixed by #1815
Assignees
Labels
bug Issue is a bug

Comments

@calldelegation
Copy link

calldelegation commented Feb 12, 2024

What version of fuels-ts are you using?

v0.73.0

Steps to Reproduce

https://github.com/calldelegation/multisig-predicate/tree/report/signing-predicate-txn

  1. Clone the repository
  2. Switch to the report/signing-predicate-txn branch
  3. Run pnpm install
  4. In another terminal run fuel-core run --db-type in-memory
  5. Run pnpm vitest --run ec-recover

Expected Behavior

In our first test, the transaction was successfully populated with the witness data using the provided lines of code. However, after making a few minor adjustments for the second test, the witness data no longer seems to populate correctly. I'm unsure if this behavior is intentional.

TEST 1: transacts using predicate
TEST 2: transacts using predicate fails
https://github.com/calldelegation/multisig-predicate/blob/report/signing-predicate-txn/frontend/tests/ec-recover.test.ts

The current naming convention adds to the confusion, leading me to believe that the transaction should automatically populate with .populateTransactionWitnessesSignature(), eliminating the need for further action on my part. Ideally, I'd prefer a more streamlined approach, perhaps integrating the signature and transaction population into a single function specifically in .signTransaction(), utilizing a Last In, First Out (LIFO) stack.

This makes it more intuitive and user-friendly.

Additionally the error message I get is as follows

Error: PredicateVerificationFailed(PanicInstruction(PanicInstruction { reason: ContractInstructionNotAllowed, instruction: RVRT { value: 0x0 } (bytes: 36 00 00 00) })): {"response":{"data":null,"errors":[{"message":"PredicateVerificationFailed(PanicInstruction(PanicInstruction { reason: ContractInstructionNotAllowed, instruction: RVRT { value: 0x0 } (bytes: 36 00 00 00) }))","locations":[{"line":2,"column":3}],"path":["submit"]}],"status":200,"headers":{}},"request":{"query":"mutation submit($encodedTransaction: HexString!) {\n  submit(tx: $encodedTransaction) {\n    id\n  }\n}","variables":{"encodedTransaction":

This error message is also very misleading. It made me think that there was an error with not the SDK but my predicate when that is not the case.

Actual Behavior

TEST 1: transacts using predicate

const signedTransaction = await coreWallet.signTransaction(request);
const transactionRequest =
  await coreWallet.populateTransactionWitnessesSignature({
    ...request,
    witnesses: [signedTransaction],
  });

returns

stdout | tests/ec-recover.test.ts > ec-recover > transacts using predicate
Witnesses [
  '0x236fa4e110d4273f78244f84e7990cf7c158ccd9745a49ab467062037134d822d115f506ef2405af94d49d426bcb8ae32ed9a1b058e12fa56c60ac876c2a5f37'
]

TEST 2: transacts using predicate fails

const signedTransaction = await coreWallet.signTransaction(request);
const transactionRequest = await coreWallet.populateTransactionWitnessesSignature(request);

returns

stdout | tests/ec-recover.test.ts > ec-recover > transacts using predicate fails
Witnesses [
  '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'
]

cc @danielbate

@calldelegation calldelegation added the bug Issue is a bug label Feb 12, 2024
@danielbate danielbate self-assigned this Feb 26, 2024
@danielbate
Copy link
Contributor

An issue here is that populateTransactionWitnessesSignature only updates a witness if a coin input belongs to the address doing the signing (I assume this is updating the dummy witness).As above, they may have no inputs, they are just there to sign the transaction, such as that as a multi sig with many signers but one coin input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants