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

txnbuild: function to verify sep10 challenge tx #1576

Merged
merged 3 commits into from
Aug 9, 2019

Conversation

poliha
Copy link
Contributor

@poliha poliha commented Aug 8, 2019

This PR adds a function VerifyChallengeTx to the txnbuild package that verifies a SEP 10 challenge transaction according to the spec.
This complements the BuildChallengeTx function which builds a challenge transaction.
This PR closes #1530

@poliha poliha added the txnbuild 2nd-generation transaction build library for Go SDK label Aug 8, 2019
@poliha poliha added this to the horizonclient 1.4.0 milestone Aug 8, 2019
Copy link
Contributor

@abuiles abuiles left a comment

Choose a reason for hiding this comment

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

@poliha LGTM!

I think it could be useful to also validate the value of the manage_data operation, that is, after reading from the base64 string, it should be 48 bytes.

txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Show resolved Hide resolved
Copy link
Member

@ire-and-curses ire-and-curses left a comment

Choose a reason for hiding this comment

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

This is mostly there, but needs some improvements.

txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction.go Show resolved Hide resolved
return verifyTxSignature(tx, serverAccountId)
}

// verifyTxSignature checks if a transaction has been signed by the provided stellar account.
Copy link
Member

Choose a reason for hiding this comment

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

stellar -> Stellar

txnbuild/transaction.go Outdated Show resolved Hide resolved
txnbuild/transaction_test.go Show resolved Hide resolved
txnbuild/transaction_test.go Show resolved Hide resolved
Copy link
Member

@ire-and-curses ire-and-curses left a comment

Choose a reason for hiding this comment

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

looks great! Just one typo

maxTime := currentTime.Add(timebound)
txTimebound = NewTimebounds(currentTime.Unix(), maxTime.Unix())
if timebound == 0 {
return "", errors.New("timebound can not be 0")
Copy link
Member

Choose a reason for hiding this comment

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

"cannot" is one word not two

@poliha poliha merged commit 2dd07b7 into release-horizonclient-v1.4.0 Aug 9, 2019
@poliha poliha deleted the verify-sep-10 branch August 9, 2019 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
txnbuild 2nd-generation transaction build library for Go SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants