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

Update docs related to transaction build process #3463

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Conversation

TalDerei
Copy link
Collaborator

@TalDerei TalDerei commented Dec 4, 2023

References #3401

transaction_signing.md doc is modified to describe the build process in more detail according to the changes in #3289. Additionally, since the UnauthTransaction type was removed, I propose changing the intermediate state of a transaction to Unauthenticated Transaction or Pending Transaction for improved clarity.

@TalDerei TalDerei self-assigned this Dec 4, 2023
@TalDerei TalDerei marked this pull request as ready for review December 4, 2023 05:51
@hdevalence
Copy link
Member

Additionally, since the UnauthTransaction type was removed, I propose changing the intermediate state of a transaction to UnauthenticatedTransaction or PendingTransaction for improved clarity.

One thing that's a little awkward is that now we don't actually have a different state, it's just a transaction (with invalid data in it). Not sure the best way to handle this in the docs.

signatures from the `AuthorizationData` are ready[^1]. This intermediate state
of the transaction without the full authorizing data is called the `UnauthTransaction`.
of the transaction without the full authorizing data is referred to as the `UnauthenticatedTransaction`.
Copy link
Member

Choose a reason for hiding this comment

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

Here we should probably say "unauthenticated Transaction", right, since there's no distinct data type / message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, changed

@TalDerei
Copy link
Collaborator Author

TalDerei commented Dec 4, 2023

Additionally, since the UnauthTransaction type was removed, I propose changing the intermediate state of a transaction to UnauthenticatedTransaction or PendingTransaction for improved clarity.

One thing that's a little awkward is that now we don't actually have a different state, it's just a transaction (with invalid data in it). Not sure the best way to handle this in the docs.

It might be better to describe it as a state transition in terms of the Transaction internals changing across the discrete build phases. I think we kind of already do that with our current description: "This intermediate state of the transaction without the full authorizing data..."?

@hdevalence hdevalence merged commit b4d65e4 into main Dec 7, 2023
5 checks passed
@hdevalence hdevalence deleted the docs-revise branch December 7, 2023 18:28
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.

2 participants