hashgraph / hashgraph/hedera-sdk-reference
Add method `addSigner` to `Transaction` and deprecate `sign` and `signWith`
- Dominant language
- HTML
- Stars
- 7
- Forks
- 3
- PR merge metrics
- No merged PRs in 30d
Description
**Existing**
```java
public final T sign(PrivateKey privateKey);
public final T signWith(PublicKey publicKey, Function transactionSigner);
```
**Issues**
- The names of these methods are _**very**_ confusing. The SDKs do not sign anything when these methods are invoked. The signing method is added to a list that eventually signs a transaction when executed.
- The `signWith` method has two arguments, one for `publicKey` and one for `transactionSigner`. This is very problematic when integrating with injected wallet software as the second argument can dynamically adapt to a new key but the first argument is static.
- It's not possible in Java to have remote/hardware signing because the interface for signing is synchronous. The JavaScript has a non-standard version of the API that returns a promise.
**Proposal**
- Remove freeze requirement for the new signer method (no signing actually takes place)
- Deprecate `sign` and `signWith`
- Introduce `SignaturePair` (name taken from header services) which is a simple type that holds a `byte[] signature` and `PublicKey publicKey`
- Introduce `TransactionSigner` and `AsyncTransactionSigner` interfaces
- Add `addSigner` and `addAsyncSigner` methods to `Transaction`
- Implement `TransactionSigner` for `PrivateKey`
```java
interface AsyncTransactionSigner {
CompletableFuture signTransactionAsync(byte[] transactionBody);
}
@FunctionalInterface
interface TransactionSigner {
SignaturePair signTransaction(byte[] transactionBody);
}
```
```java
abstract class Transaction {
public T addSigner(TransactionSigner signer);
public T addAsyncSigner(AsyncTransactionSigner signer);
}
```
---
**Migration**
Taken from an example in the Java SDK
```java
// Existing
new TokenAssociateTransaction()
.setNodeAccountIds(Collections.singletonList(response.nodeId))
.setAccountId(accountId1)
.setTokenIds(Collections.singletonList(tokenId))
.freezeWith(client)
.sign(OPERATOR_KEY)
.sign(key1)
.execute(client)
// New
new TokenAssociateTransaction()
.setNodeAccountIds(Collections.singletonList(response.nodeId))
.setAccountId(accountId1)
.setTokenIds(Collections.singletonList(tokenId))
.addSigner(OPERATOR_KEY)
.addSigner(key1)
.execute(client)
```
Majority of uses in the public use `.sign`. A `signWith` conversion is a bit more involved:
```java
// Existing
new TokenAssociateTransaction()
.setNodeAccountIds(Collections.singletonList(response.nodeId))
.setAccountId(accountId1)
.setTokenIds(Collections.singletonList(tokenId))
.freezeWith(client)
.signWith(remotePublicKey, (bodyBytes) -> remoteSign(bodyBytes))
.execute(client)
// New
new TokenAssociateTransaction()
.setNodeAccountIds(Collections.singletonList(response.nodeId))
.setAccountId(accountId1)
.setTokenIds(Collections.singletonList(tokenId))
.freezeWith(client)
.addSigner((bodyBytes) -> {
var signature = remoteSign(bodyBytes);
var publicKey = remotePublicKey();
return new SignaturePair(publicKey, signature);
})
.execute(client)
```
Contributor guide
Assessment
This issue has not been assessed yet.