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

Document or don't rename transaction fields #144

Closed
jeapostrophe opened this issue May 5, 2020 · 4 comments
Closed

Document or don't rename transaction fields #144

jeapostrophe opened this issue May 5, 2020 · 4 comments

Comments

@jeapostrophe
Copy link

jeapostrophe commented May 5, 2020

The Algorand document lists names for the fields --- https://developer.algorand.org/docs/reference/transactions/ --- and the JS SDK (maybe the other's too, but I don't know) doesn't uses these names.

For example, the JS SDK uses firstRound rather than FirstValid (field name) or fv (codec name). Basically every field has a different name.

There's no documentation that I can find anywhere that lists what the actual names the JS SDK uses. Instead, I just have to always have the source code open (https://github.com/algorand/js-algorand-sdk/blob/develop/src/transaction.ts#L70)

This is really disruptive and difficult for developing.

@algorotem algorotem self-assigned this Oct 5, 2020
@ian-algorand ian-algorand added this to the Sprint 11 milestone Oct 15, 2020
@ian-algorand ian-algorand modified the milestones: Sprint 11, Sprint 12 Nov 2, 2020
@algorotem
Copy link
Contributor

Hi @jeapostrophe Thanks for the comment and apologies for the delayed response.
The fields names in the doc you linked to are referring to the names of the fields in the actual msgpack encoding of the transaction and not in the SDKs. That being said, it's a great suggestion and we will get out a conversion table as soon as possible to make it easier.

@Eric-Warehime
Copy link
Contributor

I think it makes a lot of sense to change the naming here to be consistent with our naming spec for Transactions. But doing so would mean a breaking change. I'm adding this issue to #654 to track it whenever we want to make a major version bump.

@algoanne algoanne added Team Lamprey and removed FDE labels Apr 20, 2023
@algoanne algoanne removed this from the Sprint 12 milestone Jul 20, 2023
@algoanne
Copy link

I went through the list, here are names that are confusingly different (there are others that have some variation but semantically the same):

from - Sender
to - Receiver or AssetReceiver
firstRound - firstValid
lastRound - lastValid
freezeState - AssetFrozen
appIndex - Application ID
assetIndex - ConfigAsset (codec: caid), XferAsset (codec: xaid), FreezeAsset (codec: faid)
assetRevocationTarget - Sender when it's clawback?

It's not totally clear to me whether it's worth changing some of these now (maybe it was 3 years ago when this issue was opened). We are about to do a major version bump so if we want to change them, now would be a good time. But this will likely cause a good amount of churn for people using this SDK when they upgrade to version 3.

@algochoi
Copy link
Contributor

algochoi commented Aug 11, 2023

#804 addresses this -- we have closed the gap on naming consistencies, and the changes will go out in a breaking version bump

@algochoi algochoi reopened this Aug 11, 2023
@winder winder closed this as completed Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants