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

Duplicate close-amount/closing-amount field #971

Open
barnjamin opened this issue Apr 22, 2022 · 2 comments
Open

Duplicate close-amount/closing-amount field #971

barnjamin opened this issue Apr 22, 2022 · 2 comments
Labels
new-bug Bug report that needs triage Team Lamprey

Comments

@barnjamin
Copy link
Contributor

A community member (Frank) noticed that the closing amount field in the transaction payload

CloseAmount: uint64Ptr(stxn.ApplyData.ClosingAmount.Raw),

ClosingAmount: uint64Ptr(stxn.ClosingAmount.Raw),

Are both of these necessary?

@Eric-Warehime
Copy link
Contributor

I looked into this a bit, but we need some more investigation...

The closing amount is indeed duplicated in this API response. The cause of this is that the Transaction type in our API definition has a closing amount and potentially a payment transaction (which has a closing amount in it).

I think the fact that the txn might not have a payment txn means it's not a duplicated field 100% of the time, and I don't really see a reason to remove the closing amount from the PaymentTransaction type either.

Like I said, I just took a brief look at this so maybe there is a way we can separate these and remove one that's not immediately obvious.

@FrankSzendzielarz
Copy link

"I think the fact that the txn might not have a payment txn "

I think the issue there is that the json spec is trying to model inheritance via composition. A transaction does not 'have' a payment transaction - it either 'is' a payment transaction or is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-bug Bug report that needs triage Team Lamprey
Projects
None yet
Development

No branches or pull requests

5 participants