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

Signers were made immutable causing a breaking change #456

Closed
nkavian opened this issue Sep 24, 2022 · 4 comments · Fixed by #567
Closed

Signers were made immutable causing a breaking change #456

nkavian opened this issue Sep 24, 2022 · 4 comments · Fixed by #567
Labels

Comments

@nkavian
Copy link

nkavian commented Sep 24, 2022

What version are you using?

Java 0.29.0 upgrading to 0.37.2

What did you do?

transaction.getSignatures().clear();

What did you expect to see?

With older versions of the SDK, I could clear the signatures and perform my necessary logic.

  • Could we get a AbstractTransaction.clearSignatures() method added to the class?
  • Is there an alternate way to clear signatures?

What did you see instead?

The signatures list is now immutable. Our engineer has to arcanely copy and modify code from the SDK in order to remove the signature; which is not the right solution.

This PR #410 made the signers list immutable.

@nkavian nkavian added the bug label Sep 24, 2022
@sreuland
Copy link
Contributor

Yes, that was mentioned in the 0.32.0 release as part of overall protocol 19 updates. Has the tx instance you're working with already had sign() invoked, i.e. there are signatures in its collection? if not, it's empty, can use addSignature() as recommended for additive update to signatures collection. Other option is to rebuild the tx with TransactionBuilder, and then re-invoke the sign() methods to build out the signatures again.

@nkavian
Copy link
Author

nkavian commented Sep 26, 2022

Hi @sreuland thanks for following up. My use case is this scenario:

  • we generate an encrypted hash of the original unsigned transaction.
  • we give the encrypted hash and the unsigned transaction to the client. We don't keep a copy of the hash in our DB.
  • the client signs the request on their own system, and hands us back the encrypted hash with the signed transaction.
  • At this point, we want to clear signatures and recompute the encrypted hash, and verify if it matches the original.
    Instead of trusting one particular field transactionHash, we verify the full contents of the structure since it came from the outside. We need confirmation we're not passing along some other generated or modified transaction.

To be clear, we're not trying to add signatures directly to the list. We just want the original transaction from the signed transaction.

@secondclaw
Copy link

secondclaw commented Sep 26, 2022

We actually also had a use case where we need to add or replace signatures in an envelope, and hit a similar scenario. The solution I found (which I am not much of a fan of), is to:

  • Convert transaction to XDR --> var oldtx = tx.toEnvelopeXdr()

  • Create new V1 envelope --> var newtx = new TransactionV1Envelope()

  • Copy transaction from old envelope to new one --> newtx.setTx(oldtx.getV1().getTx())

  • Initialize a new signature array --> newtx.setSignatures(new DecoratedSignature[]{})

  • Get new transaction from XDR --> tx1= Transaction.fromV1EnvelopeXdr(newtx, network)

  • Attach signatures if needed ....

It's a bit annoying that we need do it this way, but it does work (though maybe not in your use case).

@nkavian
Copy link
Author

nkavian commented Sep 26, 2022

@secondclaw that was really helpful, we're able to avoid the longer chunk of code we had built.

I'm okay to use this workaround and I'm fine if this ticket is closed. Would still like to see if the SDK can add a clearSignatures method directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants