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

Implement partial protocol 13 support #276

Merged
merged 7 commits into from
Apr 28, 2020
Merged

Implement partial protocol 13 support #276

merged 7 commits into from
Apr 28, 2020

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Apr 22, 2020

  • Add support for new trustline flags
  • Update strkey to handle muxed accounts
  • Update Transaction class, specifically the conversion code to and from XDR

This PR still does not handle fee bump transactions and updates to SEP 10 validation. I will add support for both in the next PR.

@tamirms tamirms marked this pull request as draft April 22, 2020 19:59
@tamirms tamirms marked this pull request as ready for review April 23, 2020 11:12
src/main/java/org/stellar/sdk/StrKey.java Show resolved Hide resolved
src/main/java/org/stellar/sdk/StrKey.java Show resolved Hide resolved
src/main/java/org/stellar/sdk/StrKey.java Show resolved Hide resolved
@@ -154,28 +175,28 @@ public int getFee() {
/**
* Generates Transaction XDR object.
*/
public org.stellar.sdk.xdr.Transaction toXdr() {
public TransactionV0 toXdr() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't xdr.Transaction a default now?

Suggested change
public TransactionV0 toXdr() {
public Transaction toXdr() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

until protocol 13 is enabled the sdks should still be generating v0 transactions. We will do another release after protocol 13 is enabled so that the sdks start generating v1 transactions. But, this method should be private the SDK users can get the xdr envelope. they don't need a public method to get the transaction from the envelope. I will change this method to private

Choose a reason for hiding this comment

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

Our previous code using stellar api is broken now. I was told to use the latest version. One of place broken is that we were getting transaction from transaction envelope. Obviouly, it is broken now. Could you please let me know what is the alternative way to get the transaction from the envelope with current version? Are we suppose to keep compatability in version upgrading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wumingmo can you please create an issue and provide some example code which demonstrates the broken behavior? thanks!

Choose a reason for hiding this comment

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

Thanks, Tamirms, for quick reponse, I created a pull request before, #284, but was closed and told to use the latest version. Of course, after I use the 0.16.0, I got the compilation error. If possible, could you please let me know how to extract the transaction from the envelope with 0.16.0? Thanks

if (encoded.length == 0) {
throw new IllegalArgumentException("Encoded char array cannot be empty.");
}

byte[] bytes = new byte[encoded.length];
for (int i = 0; i < encoded.length; i++) {
if (encoded[i] > 127) {

Choose a reason for hiding this comment

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

Not relevant to this PR, but why is this necesary? base32Encoding.decode should be taking care of this, shouldn't it?

Choose a reason for hiding this comment

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

Maybe this is needed due to guava not handling it properly ...

@tamirms tamirms merged commit 549ae54 into release-0.16.0 Apr 28, 2020
@tamirms tamirms deleted the update-xdr branch April 28, 2020 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants