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

Add support for BumpFootprintExpirationOperation and RestoreFootprintOperation #491

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.stellar.sdk;

import lombok.EqualsAndHashCode;
import lombok.NonNull;
import lombok.Value;
import lombok.experimental.SuperBuilder;
import org.stellar.sdk.xdr.BumpFootprintExpirationOp;
import org.stellar.sdk.xdr.ExtensionPoint;
import org.stellar.sdk.xdr.OperationType;
import org.stellar.sdk.xdr.Uint32;

/**
* Represents <a
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/txnbuild/bump_footprint_expiration.go#L8-L12"
* target="_blank">BumpFootprintExpiration</a> operation.
*
* <p>Bump the expiration of a footprint (read and written ledger keys).
*
* @see <a href="https://developers.stellar.org/docs/fundamentals-and-concepts/list-of-operations"
* target="_blank">List of Operations</a>
*/
@EqualsAndHashCode(callSuper = true)
@SuperBuilder(toBuilder = true)
@Value
public class BumpFootprintExpirationOperation extends Operation {
Copy link
Contributor

@sreuland sreuland Jul 27, 2023

Choose a reason for hiding this comment

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

can you add TODO's on these that state invoke, bump, restore soroban op classes in this package still require more infrastructure before they can be used in TransactionBuilder and submission, they can't be added as an Operation in a standard 'classic' tx.

Since they have to go through preflight with rpc first, to define footprint, fees and then build out a single operation tx with all that. the js sdk, js-soroban-client, has added this functionality for tx sub of soroban ops which can be used as a reference later on for same effort here. I created new ticket to capture this effort - #495

I updated and commented on #486 to reflect the narrowing of scope for this PR to be primarily for the response object marshaling.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have decided not to modify it for now, see #491 (comment)

Copy link
Contributor

@sreuland sreuland Jul 27, 2023

Choose a reason for hiding this comment

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

ok, I think I understand, you will leave the classes in this pr, and once it merges to soroban, then the wip there in #492 will refer to them?


/**
* the number of ledgers past the LCL (last closed ledger) by which to extend the validity of the
* ledger keys in this transaction
*/
@NonNull Integer ledgersToExpire;

/**
* Constructs a new BumpFootprintExpirationOperation object from the XDR representation of the
* {@link BumpFootprintExpirationOperation}.
*
* @param op the XDR representation of the {@link BumpFootprintExpirationOperation}.
*/
public static BumpFootprintExpirationOperation fromXdr(BumpFootprintExpirationOp op) {
return BumpFootprintExpirationOperation.builder()
.ledgersToExpire(op.getLedgersToExpire().getUint32())
.build();
}

@Override
org.stellar.sdk.xdr.Operation.OperationBody toOperationBody(AccountConverter accountConverter) {
BumpFootprintExpirationOp op = new BumpFootprintExpirationOp();
op.setExt(new ExtensionPoint.Builder().discriminant(0).build());
op.setLedgersToExpire(new Uint32(ledgersToExpire));

org.stellar.sdk.xdr.Operation.OperationBody body =
new org.stellar.sdk.xdr.Operation.OperationBody();
body.setDiscriminant(OperationType.BUMP_FOOTPRINT_EXPIRATION);
body.setBumpFootprintExpirationOp(op);
return body;
}

/** Customizing builder methods. Rest of the builder code will be auto generated by Lombok. */
public abstract static class BumpFootprintExpirationOperationBuilder<
C extends BumpFootprintExpirationOperation,
B extends BumpFootprintExpirationOperationBuilder<C, B>>
extends OperationBuilder<C, B> {
public B ledgersToExpire(Integer ledgersToExpire) {
if (ledgersToExpire <= 0) {
throw new IllegalArgumentException("ledgersToExpire isn't a ledger quantity (uint32)");
}
this.ledgersToExpire = ledgersToExpire;
return self();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

/**
* Represents <a
* href="https://developers.stellar.org/docs/fundamentals-and-concepts/list-of-operations#invoke-host-function"
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/txnbuild/invoke_host_function.go#L8-L13"
* target="_blank">InvokeHostFunction</a> operation.
*
* @see <a href="https://developers.stellar.org/docs/fundamentals-and-concepts/list-of-operations"
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/stellar/sdk/Operation.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ public static Operation fromXdr(
case INVOKE_HOST_FUNCTION:
operation = InvokeHostFunctionOperation.fromXdr(body.getInvokeHostFunctionOp());
break;
case BUMP_FOOTPRINT_EXPIRATION:
operation = BumpFootprintExpirationOperation.fromXdr(body.getBumpFootprintExpirationOp());
sreuland marked this conversation as resolved.
Show resolved Hide resolved
break;
case RESTORE_FOOTPRINT:
operation = RestoreFootprintOperation.fromXdr(body.getRestoreFootprintOp());
break;
default:
throw new RuntimeException("Unknown operation body " + body.getDiscriminant());
}
Expand Down
46 changes: 46 additions & 0 deletions src/main/java/org/stellar/sdk/RestoreFootprintOperation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.stellar.sdk;

import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import lombok.Value;
import lombok.experimental.SuperBuilder;
import org.stellar.sdk.xdr.ExtensionPoint;
import org.stellar.sdk.xdr.OperationType;
import org.stellar.sdk.xdr.RestoreFootprintOp;

/**
* Represents <a
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/txnbuild/restore_footprint.go#L8-L11"
* target="_blank">RestoreFootprint</a> operation.
*
* @see <a href="https://developers.stellar.org/docs/fundamentals-and-concepts/list-of-operations"
* target="_blank">List of Operations</a>
*/
@EqualsAndHashCode(callSuper = true)
@SuperBuilder(toBuilder = true)
@Value
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class RestoreFootprintOperation extends Operation {
/**
* Constructs a new RestoreFootprintOperation object from the XDR representation of the {@link
* RestoreFootprintOperation}.
*
* @param op the XDR representation of the {@link RestoreFootprintOperation}.
*/
public static RestoreFootprintOperation fromXdr(RestoreFootprintOp op) {
return new RestoreFootprintOperation();
}

@Override
org.stellar.sdk.xdr.Operation.OperationBody toOperationBody(AccountConverter accountConverter) {
RestoreFootprintOp op = new RestoreFootprintOp();
op.setExt(new ExtensionPoint.Builder().discriminant(0).build());

org.stellar.sdk.xdr.Operation.OperationBody body =
new org.stellar.sdk.xdr.Operation.OperationBody();
body.setDiscriminant(OperationType.RESTORE_FOOTPRINT);
body.setRestoreFootprintOp(op);
return body;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ public OperationResponse deserialize(
return gson.fromJson(json, LiquidityPoolWithdrawOperationResponse.class);
case INVOKE_HOST_FUNCTION:
return gson.fromJson(json, InvokeHostFunctionOperationResponse.class);
case BUMP_FOOTPRINT_EXPIRATION:
return gson.fromJson(json, BumpFootprintExpirationOperationResponse.class);
case RESTORE_FOOTPRINT:
return gson.fromJson(json, RestoreFootprintOperationResponse.class);
default:
throw new RuntimeException("Invalid operation type");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.stellar.sdk.responses.operations;

import com.google.gson.annotations.SerializedName;

/**
* Represents BumpFootprintExpiration operation response.
*
* @see <a
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/protocols/horizon/operations/main.go#L376-L381">Horizon
* Protocol</a>
* @see <a href="https://developers.stellar.org/api/horizon/resources/operations"
* target="_blank">Operation documentation</a>
* @see org.stellar.sdk.requests.OperationsRequestBuilder
* @see org.stellar.sdk.Server#operations()
*/
public class BumpFootprintExpirationOperationResponse extends OperationResponse {
@SerializedName("ledgers_to_expire")
private final Long ledgersToExpire;

public BumpFootprintExpirationOperationResponse(Long ledgersToExpire) {
this.ledgersToExpire = ledgersToExpire;
}

public Long getLedgersToExpire() {
return ledgersToExpire;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
/**
* Represents InvokeHostFunction operation response.
*
* @see <a
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/protocols/horizon/operations/main.go#L350-L367">Horizon
* Protocol</a>
* @see <a href="https://developers.stellar.org/api/horizon/resources/operations"
* target="_blank">Operation documentation</a>
* @see org.stellar.sdk.requests.OperationsRequestBuilder
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.stellar.sdk.responses.operations;

/**
* Represents RestoreFootprint operation response.
*
* @see <a
* href="https://github.com/stellar/go/blob/7ff6ffae29d278f979fcd6c6bed8cd0d4b4d2e08/protocols/horizon/operations/main.go#L383-L386">Horizon
* Protocol</a>
* @see <a href="https://developers.stellar.org/api/horizon/resources/operations"
* target="_blank">Operation documentation</a>
* @see org.stellar.sdk.requests.OperationsRequestBuilder
* @see org.stellar.sdk.Server#operations()
*/
public class RestoreFootprintOperationResponse extends OperationResponse {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package org.stellar.sdk;

import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNull;

import org.junit.Assert;
import org.junit.Test;

public class BumpFootprintExpirationOperationTest {
@Test
public void testBuilderWithSource() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
BumpFootprintExpirationOperation op =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
assertEquals(Integer.valueOf(123), op.getLedgersToExpire());
assertEquals(source, op.getSourceAccount());
String expectXdr = "AAAAAQAAAAAk4TTtavBWsGnEN3KxHw4Ohwi22ZJHWi8hlamN5pm0TgAAABkAAAAAAAAAew==";
Copy link
Contributor

Choose a reason for hiding this comment

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

are there other tests in this repo that hardcode expected xdr, that may not be a good pattern to follow, can make the test fragile as the xdr may change, expand over time. Instead, can do a full round trip with marshaling such as secondOp = Operation.fromXdr(op.toXdrBase64()) and then assertEquals(secondOp, op)

Copy link
Member Author

Choose a reason for hiding this comment

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

I tend to keep it. Our XDR module code lacks sufficient testing, and these XDR values are generated by other SDKs (Python, JS), which can serve as cross-validation. Additionally, changes in XDR definitions do not occur frequently, so even if we update it with some effort, it is acceptable.

In the previous code, there were also some validations done in this way. https://github.com/stellar/java-stellar-sdk/blob/master/src/test/java/org/stellar/sdk/OperationTest.java#L167

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I have seen the usage of hardcoded xdr for serialization tests, not suggesting to change either, but that is more for scope in the xdrgen library to verify. This unit test's responsibility is to validate the code from this sdk builds a valid java object graph for BumpFootprint, and it has asserted that, it doesn't seem like the extra assertions on serialized xdr are related to code in this repo. But, I am relatively newer to this repo, and would be great if @tamirms wants to jump in with any thoughts on this also.

assertEquals(expectXdr, op.toXdrBase64());
}

@Test
public void testBuilderWithoutSource() {
BumpFootprintExpirationOperation op =
BumpFootprintExpirationOperation.builder().ledgersToExpire(123).build();
assertEquals(Integer.valueOf(123), op.getLedgersToExpire());
assertNull(op.getSourceAccount());
String expectXdr = "AAAAAAAAABkAAAAAAAAAew==";
assertEquals(expectXdr, op.toXdrBase64());
}

@Test
public void testFromXdr() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
BumpFootprintExpirationOperation originOp =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
org.stellar.sdk.xdr.Operation xdrObject = originOp.toXdr();
Operation restartOp = Operation.fromXdr(xdrObject);
Assert.assertEquals(restartOp, originOp);
}

@Test
public void testEquals() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
BumpFootprintExpirationOperation operation1 =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
BumpFootprintExpirationOperation operation2 =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
assertEquals(operation1, operation2);
}

@Test
public void testNotEquals() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
BumpFootprintExpirationOperation operation1 =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
BumpFootprintExpirationOperation operation2 =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(124)
.sourceAccount(source)
.build();
Assert.assertNotEquals(operation1, operation2);
}

@Test
public void testNotEqualsSource() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
BumpFootprintExpirationOperation operation1 =
BumpFootprintExpirationOperation.builder()
.ledgersToExpire(123)
.sourceAccount(source)
.build();
BumpFootprintExpirationOperation operation2 =
BumpFootprintExpirationOperation.builder().ledgersToExpire(123).build();
Assert.assertNotEquals(operation1, operation2);
}

@Test
public void testLedgersToExpireIsInvalidThrows() {
try {
BumpFootprintExpirationOperation.builder().ledgersToExpire(-1).build();
Assert.fail();
} catch (IllegalArgumentException e) {
Assert.assertEquals("ledgersToExpire isn't a ledger quantity (uint32)", e.getMessage());
}
}

@Test
public void testLedgersToExpireIsNullThrows() {
try {
BumpFootprintExpirationOperation.builder().build();
Assert.fail();
} catch (NullPointerException e) {
Assert.assertEquals("ledgersToExpire is marked non-null but is null", e.getMessage());
}
}
}
56 changes: 56 additions & 0 deletions src/test/java/org/stellar/sdk/RestoreFootprintOperationTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package org.stellar.sdk;

import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNull;

import org.junit.Assert;
import org.junit.Test;

public class RestoreFootprintOperationTest {
@Test
public void testBuilderWithSource() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
RestoreFootprintOperation op =
RestoreFootprintOperation.builder().sourceAccount(source).build();
assertEquals(source, op.getSourceAccount());
String expectXdr = "AAAAAQAAAAAk4TTtavBWsGnEN3KxHw4Ohwi22ZJHWi8hlamN5pm0TgAAABoAAAAA";
assertEquals(expectXdr, op.toXdrBase64());
}

@Test
public void testBuilderWithoutSource() {
RestoreFootprintOperation op = RestoreFootprintOperation.builder().build();
assertNull(op.getSourceAccount());
String expectXdr = "AAAAAAAAABoAAAAA";
assertEquals(expectXdr, op.toXdrBase64());
}

@Test
public void testFromXdr() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
RestoreFootprintOperation originOp =
RestoreFootprintOperation.builder().sourceAccount(source).build();
org.stellar.sdk.xdr.Operation xdrObject = originOp.toXdr();
Operation restartOp = Operation.fromXdr(xdrObject);
Assert.assertEquals(restartOp, originOp);
}

@Test
public void testEquals() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
RestoreFootprintOperation operation1 =
RestoreFootprintOperation.builder().sourceAccount(source).build();
RestoreFootprintOperation operation2 =
RestoreFootprintOperation.builder().sourceAccount(source).build();
assertEquals(operation1, operation2);
}

@Test
public void testNotEquals() {
String source = "GASOCNHNNLYFNMDJYQ3XFMI7BYHIOCFW3GJEOWRPEGK2TDPGTG2E5EDW";
RestoreFootprintOperation operation1 =
RestoreFootprintOperation.builder().sourceAccount(source).build();
RestoreFootprintOperation operation2 = RestoreFootprintOperation.builder().build();
Assert.assertNotEquals(operation1, operation2);
}
}
Loading
Loading