hashgraph / hashgraph/hedera-sdk-reference

Add method `addSigner` to `Transaction` and deprecate `sign` and `signWith`

Open
#55 0 comments 2 reactions 3 assignees Claimed by @janaakhterov View on GitHub
enhancement
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.