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.

Sign up for GitHub

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

Jump to bottom

Update XDR for Protocol 19, add encoding support for CAP-40 SignedPayload Signer #413

Closed
wants to merge 7 commits into from
Closed

Update XDR for Protocol 19, add encoding support for CAP-40 SignedPayload Signer #413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90f3ad5
#410: updated xdr definitions for Protocol 19, add SignedPayload Sign…
sreuland Mar 21, 2022
5bf518b
#410: use xdr data types for predicate time dimensions, incorporate s…
sreuland Mar 21, 2022
c44b1de
#410: use xdr encoding/decoding for signed payload (de)ser, use Accou…
sreuland Mar 22, 2022
163b582
#410: maintain backwards compliant Predicate
sreuland Mar 22, 2022
1717767
#410: renamed parameter to 'signer' per feedback on pr
sreuland Mar 23, 2022
49b477d
#410: regenerated all xdr from latest .x source on p19 branch
sreuland Mar 23, 2022
c61a5d5
#410: consolidate validation of signed payload aspects into SignedPay…
sreuland Mar 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

As this project is pre 1.0, breaking changes may happen for minor version bumps. A breaking change will get clearly notified in this log.

## 0.32.0 (Pending)

* Update XDR definitions and auto-generated classes to support upcoming protocol 19 release ([#276](https://github.com/stellar/java-stellar-sdk/pull/276)).
* Extend StrKey implementation to handle [CAP 40 Payload Signer](https://github.com/stellar/stellar-protocol/blob/master/core/cap-0040.md) ([#276](https://github.com/stellar/java-stellar-sdk/pull/276)).


## 0.31.0

* Fixed NPE on TrustlineCreatedEffectResponse.getAsset() for liquidity pool asset type.
Expand Down
6 changes: 6 additions & 0 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,9 @@ For information on how to contribute, please refer to our [contribution guide](h

## License
java-stellar-sdk is licensed under an Apache-2.0 license. See the [LICENSE](https://github.com/stellar/java-stellar-sdk/blob/master/LICENSE) file for details.

## xdr to jave code generation
All the java source files in org.stellar.sdk.xdr package are generated using the `xdrgen` command from the [stellar/xdrgen](https://github.com/stellar/xdrgen)
```
xdrgen -o ./src/main/java/org/stellar/sdk/xdr -l java -n org.stellar.sdk.xdr ./xdr/Stellar-types.x ./xdr/Stellar-SCP.x ./xdr/Stellar-overlay.x ./xdr/Stellar-ledger-entries.x ./xdr/Stellar-ledger.x ./xdr/Stellar-transaction.x
```
19 changes: 13 additions & 6 deletions src/main/java/org/stellar/sdk/Predicate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import com.google.common.collect.Lists;
import org.stellar.sdk.xdr.ClaimPredicate;
import org.stellar.sdk.xdr.ClaimPredicateType;
import org.stellar.sdk.xdr.Duration;
import org.stellar.sdk.xdr.Int64;
import org.stellar.sdk.xdr.TimePoint;
import org.stellar.sdk.xdr.Uint64;
import org.threeten.bp.Instant;

import java.util.List;
Expand Down Expand Up @@ -34,9 +37,9 @@ public static Predicate fromXdr(org.stellar.sdk.xdr.ClaimPredicate xdr) {
case CLAIM_PREDICATE_NOT:
return new Not(fromXdr(xdr.getNotPredicate()));
case CLAIM_PREDICATE_BEFORE_RELATIVE_TIME:
return new RelBefore(xdr.getRelBefore().getInt64());
return new RelBefore(xdr.getRelBefore().getDuration().getInt64());
sreuland marked this conversation as resolved.
Show resolved Hide resolved
case CLAIM_PREDICATE_BEFORE_ABSOLUTE_TIME:
return new AbsBefore(xdr.getAbsBefore().getInt64());
return new AbsBefore(xdr.getAbsBefore().getTimePoint().getUint64());
default:
throw new IllegalArgumentException("Unknown asset type " + xdr.getDiscriminant());
}
Expand Down Expand Up @@ -209,9 +212,11 @@ public int hashCode() {
public ClaimPredicate toXdr() {
org.stellar.sdk.xdr.ClaimPredicate xdr = new org.stellar.sdk.xdr.ClaimPredicate();
xdr.setDiscriminant(ClaimPredicateType.CLAIM_PREDICATE_BEFORE_ABSOLUTE_TIME);
Int64 t = new Int64();
t.setInt64(epochSeconds);
xdr.setAbsBefore(t);
Uint64 t = new Uint64();
t.setUint64(epochSeconds);
TimePoint beforeTime = new TimePoint();
beforeTime.setTimePoint(t);
xdr.setAbsBefore(beforeTime);
return xdr;
}
}
Expand Down Expand Up @@ -246,7 +251,9 @@ public ClaimPredicate toXdr() {
xdr.setDiscriminant(ClaimPredicateType.CLAIM_PREDICATE_BEFORE_RELATIVE_TIME);
Int64 t = new Int64();
t.setInt64(secondsSinceClose);
xdr.setRelBefore(t);
Duration beforeTime = new Duration();
beforeTime.setDuration(t);
xdr.setRelBefore(beforeTime);
return xdr;
}
}
Expand Down
45 changes: 45 additions & 0 deletions src/main/java/org/stellar/sdk/SignedPayloadSigner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.stellar.sdk;

/**
* Data model for the <a href="https://github.com/stellar/stellar-protocol/blob/master/core/cap-0040.md#xdr-changes">signed payload signer </a>
*/
public class SignedPayloadSigner {
private String accountId;
private byte[] payload;

/**
* constructor
*
* @param accountId - the StrKey format of a stellar AccountId
* @param payload - the raw payload for a signed payload
*/
public SignedPayloadSigner(String accountId, byte[] payload) {
this.accountId = accountId;
this.payload = payload;
}

/**
* get the StrKey encoded representation of a stellar account id
sreuland marked this conversation as resolved.
Show resolved Hide resolved
* @return stellar account id in StrKey encoding
*/
public String getEncodedAccountId() {
return accountId;
}

/**
* get the binary format of a stellar account id, a Ed25519 public key
* @return stellar account id in binary format
*/
public byte[] getDecodedAccountId() {
return StrKey.decodeStellarAccountId(accountId);
}
sreuland marked this conversation as resolved.
Show resolved Hide resolved

/**
* get the payload that signatures are produced from.
* @see <a href="https://github.com/stellar/stellar-protocol/blob/master/core/cap-0040.md#semantics"/>
* @return
*/
public byte[] getPayload() {
return payload;
}
}
24 changes: 24 additions & 0 deletions src/main/java/org/stellar/sdk/Signer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
import org.stellar.sdk.xdr.SignerKeyType;
import org.stellar.sdk.xdr.Uint256;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

/**
* Signer is a helper class that creates {@link org.stellar.sdk.xdr.SignerKey} objects.
*/
public class Signer {
public static final int SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH = 64;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, I think this belongs above in SignedPayloadSigner

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I had located validation here in the Signer.signedPayload(SignedPayloadSigner) factory method which constructs the xdr SignerKey for payload signer from the payload. The intent on SignedPayloadSigner was to be data transfer object, no logic, to your point, it could take on that role also of validation, but thought was keeping that logic closer to when it's used in SignerKey, let me know if you want to discuss more, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do have validation in two places (signer key and strkey), and imo you'd never want a SignedPayloadSigner that wasn't valid, but your separation of concerns also makes sense. No strong opinion here:+1:


/**
* Create <code>ed25519PublicKey</code> {@link org.stellar.sdk.xdr.SignerKey} from
* a {@link org.stellar.sdk.KeyPair}
Expand Down Expand Up @@ -72,6 +75,27 @@ public static SignerKey preAuthTx(byte[] hash) {
return signerKey;
}

/**
* Create <code>SignerKey</code> {@link org.stellar.sdk.xdr.SignerKey} from {@link org.stellar.sdk.SignedPayloadSigner}
*
* @param signedPayloadSigner - signed payload values
* @return org.stellar.sdk.xdr.SignerKey
*/
public static SignerKey signedPayload(SignedPayloadSigner signedPayloadSigner) {
checkNotNull(signedPayloadSigner.getEncodedAccountId(), "accountId cannot be null");
checkArgument(signedPayloadSigner.getPayload().length <= SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH );

SignerKey signerKey = new SignerKey();
SignerKey.SignerKeyEd25519SignedPayload payloadSigner = new SignerKey.SignerKeyEd25519SignedPayload();
payloadSigner.setPayload(signedPayloadSigner.getPayload());
payloadSigner.setEd25519(createUint256(signedPayloadSigner.getDecodedAccountId()));

signerKey.setDiscriminant(SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD);
signerKey.setEd25519SignedPayload(payloadSigner);

return signerKey;
}

private static Uint256 createUint256(byte[] hash) {
if (hash.length != 32) {
throw new RuntimeException("hash must be 32 bytes long");
Expand Down
73 changes: 62 additions & 11 deletions src/main/java/org/stellar/sdk/StrKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
package org.stellar.sdk;

import com.google.common.io.BaseEncoding;
import com.google.common.base.Optional;
import com.google.common.io.BaseEncoding;
import com.google.common.primitives.Bytes;
import com.google.common.primitives.Longs;
import org.stellar.sdk.xdr.*;

import java.io.*;
import org.stellar.sdk.xdr.AccountID;
import org.stellar.sdk.xdr.CryptoKeyType;
import org.stellar.sdk.xdr.MuxedAccount;
import org.stellar.sdk.xdr.PublicKey;
import org.stellar.sdk.xdr.PublicKeyType;
import org.stellar.sdk.xdr.Uint256;
import org.stellar.sdk.xdr.Uint64;
import org.stellar.sdk.xdr.XdrDataInputStream;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.CharArrayWriter;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.Arrays;

import static org.stellar.sdk.Signer.SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH;

class StrKey {

public static final int ACCOUNT_ID_ADDRESS_LENGTH = 56;
Expand All @@ -18,7 +33,8 @@ public enum VersionByte {
MUXED((byte)(12 << 3)), // M
SEED((byte)(18 << 3)), // S
PRE_AUTH_TX((byte)(19 << 3)), // T
SHA256_HASH((byte)(23 << 3)); // X
SHA256_HASH((byte)(23 << 3)), // X
SIGNED_PAYLOAD((byte)(15 << 3)); // P
private final byte value;
VersionByte(byte value) {
this.value = value;
Expand Down Expand Up @@ -49,6 +65,25 @@ public static String encodeStellarAccountId(AccountID accountID) {
return String.valueOf(encoded);
}

public static String encodeSignedPayload(SignedPayloadSigner signedPayloadSigner) {
if (signedPayloadSigner.getPayload().length > SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH) {
throw new FormatException("invalid payload length, must be less than " + SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH);
}
try {
ByteArrayOutputStream record = new ByteArrayOutputStream();
DataOutputStream dataStream = new DataOutputStream(record);
dataStream.write(signedPayloadSigner.getDecodedAccountId());
dataStream.writeInt(signedPayloadSigner.getPayload().length);
dataStream.write(signedPayloadSigner.getPayload());
int padding = signedPayloadSigner.getPayload().length % 4 > 0 ? 4 - signedPayloadSigner.getPayload().length % 4 : 0;
dataStream.write(new byte[padding]);
char[] encoded = encodeCheck(VersionByte.SIGNED_PAYLOAD, record.toByteArray());
return String.valueOf(encoded);
sreuland marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception ex) {
throw new FormatException(ex.getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why rethrow when you can just let it bubble up?

Copy link
Contributor Author

@sreuland sreuland Mar 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to muffle the checked exceptions which otherwise would require adding on a throws declaration on the method for each one, which then forces the callers to specifically handle it, it's mostly for caller convenience as there is no significant app-level meaning to a checked failure that the client could catch and do something else instead, rather they should just let it go up also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, java-isms.. 👍

}
}

public static String encodeStellarMuxedAccount(MuxedAccount muxedAccount) {
switch (muxedAccount.getDiscriminant()) {
case KEY_TYPE_MUXED_ED25519:
Expand Down Expand Up @@ -154,6 +189,22 @@ public static byte[] decodeStellarSecretSeed(char[] data) {
return decodeCheck(VersionByte.SEED, data);
}

public static SignedPayloadSigner decodeSignedPayload(char[] data) {
try {
byte[] signedPayloadRaw = decodeCheck(VersionByte.SIGNED_PAYLOAD, data);
DataInputStream dataStream = new DataInputStream(new ByteArrayInputStream(signedPayloadRaw));
byte[] binaryAccountId = new byte[32];
dataStream.read(binaryAccountId);
int payloadLength = dataStream.readInt();
byte[] payload = new byte[payloadLength];
dataStream.read(payload);

return new SignedPayloadSigner(encodeStellarAccountId(binaryAccountId), payload );
} catch (Exception ex) {
throw new FormatException(ex.getMessage());
}
}

public static String encodePreAuthTx(byte[] data) {
char[] encoded = encodeCheck(VersionByte.PRE_AUTH_TX, data);
return String.valueOf(encoded);
Expand All @@ -177,20 +228,20 @@ protected static char[] encodeCheck(VersionByte versionByte, byte[] data) {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
outputStream.write(versionByte.getValue());
outputStream.write(data);
byte payload[] = outputStream.toByteArray();
byte checksum[] = StrKey.calculateChecksum(payload);
byte[] payload = outputStream.toByteArray();
byte[] checksum = StrKey.calculateChecksum(payload);
outputStream.write(checksum);
byte unencoded[] = outputStream.toByteArray();
byte[] unencoded = outputStream.toByteArray();

if (VersionByte.SEED != versionByte) {
return StrKey.base32Encoding.encode(unencoded).toCharArray();
return base32Encoding.encode(unencoded).toCharArray();
}

// Why not use base32Encoding.encode here?
// We don't want secret seed to be stored as String in memory because of security reasons. It's impossible
// to erase it from memory when we want it to be erased (ASAP).
CharArrayWriter charArrayWriter = new CharArrayWriter(unencoded.length);
OutputStream charOutputStream = StrKey.base32Encoding.encodingStream(charArrayWriter);
OutputStream charOutputStream = base32Encoding.encodingStream(charArrayWriter);
Comment on lines 233 to +237
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errm, note the comment above this line? I think there's a reason we use StrKey? Maybe @tamirms knows more on this (or git blame it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment was referring to the usage of .encode() vs .encodingStream(), the change here was just source tidy-up, StrKey.base32Encoding was redundant, as within StrKey class scope, it references the same static member declared here at base32Encoding

charOutputStream.write(unencoded);
char[] charsEncoded = charArrayWriter.toCharArray();

Expand Down Expand Up @@ -242,7 +293,7 @@ protected static byte[] decodeCheck(VersionByte versionByte, char[] encoded) {
}
}

byte[] decoded = StrKey.base32Encoding.decode(java.nio.CharBuffer.wrap(encoded));
byte[] decoded = base32Encoding.decode(java.nio.CharBuffer.wrap(encoded));
byte decodedVersionByte = decoded[0];
byte[] payload = Arrays.copyOfRange(decoded, 0, decoded.length-2);
byte[] data = Arrays.copyOfRange(payload, 1, payload.length);
Expand Down
29 changes: 26 additions & 3 deletions src/main/java/org/stellar/sdk/Transaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
package org.stellar.sdk;

import com.google.common.base.Objects;
import org.stellar.sdk.xdr.*;
import org.stellar.sdk.xdr.ClaimableBalanceID;
import org.stellar.sdk.xdr.ClaimableBalanceIDType;
import org.stellar.sdk.xdr.DecoratedSignature;
import org.stellar.sdk.xdr.EnvelopeType;
import org.stellar.sdk.xdr.Hash;
import org.stellar.sdk.xdr.Int64;
import org.stellar.sdk.xdr.OperationID;
import org.stellar.sdk.xdr.PreconditionType;
import org.stellar.sdk.xdr.Preconditions;
import org.stellar.sdk.xdr.SequenceNumber;
import org.stellar.sdk.xdr.TransactionEnvelope;
import org.stellar.sdk.xdr.TransactionSignaturePayload;
import org.stellar.sdk.xdr.TransactionV0;
import org.stellar.sdk.xdr.TransactionV0Envelope;
import org.stellar.sdk.xdr.TransactionV1Envelope;
import org.stellar.sdk.xdr.Uint32;
import org.stellar.sdk.xdr.XdrDataOutputStream;
sreuland marked this conversation as resolved.
Show resolved Hide resolved

import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand Down Expand Up @@ -188,7 +204,11 @@ private org.stellar.sdk.xdr.Transaction toV1Xdr(AccountConverter accountConverte
v1Tx.setSourceAccount(accountConverter.encode(mSourceAccount));
v1Tx.setOperations(operations);
v1Tx.setMemo(mMemo.toXdr());
v1Tx.setTimeBounds(mTimeBounds == null ? null : mTimeBounds.toXdr());
Preconditions.Builder preconditions = new Preconditions.Builder().discriminant(PreconditionType.PRECOND_NONE);
if (mTimeBounds != null) {
preconditions.discriminant(PreconditionType.PRECOND_TIME).timeBounds(mTimeBounds.toXdr());
}
v1Tx.setCond(preconditions.build());
v1Tx.setExt(ext);

return v1Tx;
Expand Down Expand Up @@ -228,9 +248,12 @@ public static Transaction fromV0EnvelopeXdr(TransactionV0Envelope envelope, Netw

public static Transaction fromV1EnvelopeXdr(AccountConverter accountConverter, TransactionV1Envelope envelope, Network network) {
int mFee = envelope.getTx().getFee().getUint32();
TimeBounds mTimeBounds = null;
Long mSequenceNumber = envelope.getTx().getSeqNum().getSequenceNumber().getInt64();
Memo mMemo = Memo.fromXdr(envelope.getTx().getMemo());
TimeBounds mTimeBounds = TimeBounds.fromXdr(envelope.getTx().getTimeBounds());
if (envelope.getTx().getCond().getDiscriminant().equals(PreconditionType.PRECOND_TIME)) {
mTimeBounds = TimeBounds.fromXdr(envelope.getTx().getCond().getTimeBounds());
}
Comment on lines +254 to +256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary but not sufficient: there can also be timebounds on the PRECOND_V2 type. maybe a helper is in order for a general way to retrieve timebounds given a tx?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I'll use that suggestion, will be in the next follow-on PR that adds the CAP21 support on those new conditions, I included all the xdr schema changes up front in this pr, but only did the cap40 changes above first, this part ideally was just trying to refactor to maintain present compile/test until then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a way to do it here actually would be to say instead .notEquals(PreconditionType.PRECOND_NONE), since both other forms have a timebound condition 🤔


Operation[] mOperations = new Operation[envelope.getTx().getOperations().length];
for (int i = 0; i < envelope.getTx().getOperations().length; i++) {
Expand Down
Loading