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.

Sign up for GitHub

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

Jump to bottom

Potentially remove the afterSign hook point #13

Closed
aaroncox opened this issue Dec 7, 2022 · 1 comment
Closed

Potentially remove the afterSign hook point #13

aaroncox opened this issue Dec 7, 2022 · 1 comment
Milestone
0.2.x

Comments

@aaroncox
Copy link
Member

aaroncox commented Dec 7, 2022

Both the afterSign and beforeBroadcast hooks basically do the same thing, can we remove/merge them?
@aaroncox aaroncox added this to the 0.2.0 milestone Dec 21, 2022
@shaqk
Copy link

shaqk commented Jan 10, 2023

I would keep afterSign, and remove beforeBroadcast.
Not all txs will be broadcasted but one may still want to leverage afterSign in them

aaroncox added a commit that referenced this issue Jan 17, 2023
@aaroncox
0.2.0 - Release (#20)
26f4dea 
* Removal of the beforeBroadcast hook.

Resolves #13.

After discussions with other developers, it became clear that the `afterSign` and `beforeBroadcast` hooks currently serve the same function. To simplify the approach developers can take for plugin development, the `beforeBroadcast` hook was removed, and if required, can be added back at a later date.

* Added default esrOptions to the TransactContext

Resolves #15.

This gives developers a default set of options when creating ESR requests.

* Renamed Context `session` to `permissionLevel`

* Renamed getters to be more specific

Motivation was that I imagine we'll want the `.account` getter to return an Account Kit instance.

* Added getters to the TransactContext

* Deduplicating code by adding a helper function

* Added the ABICache and implemented in the TransactContext

* Removed unused import

* Allow specifying `expireSeconds` throughout the stack

Resolves #22

* An ENV variable to dump http request traffic for debugging

* Add creation of a wharfkitnoop account for cosigning tests

* Ignore any local notes

* Refector TransactContext and ABICache

* Allow context to resolve transactions (ease for developers)

* Retain a revision history of request modifications in the TransactResult

This will

* Removed unused import

* Fix for nodejs v14

Cannot use the at method in older nodejs versions

* Manually clone incoming SigningRequest

This is to preserve all data and override the zlib/abiProvider values.

* Migrate plugin tests into new suite

* Split cloneRequest into its own function

* Added and implemented updateRequest method

Resolves #18.

This new update method ensures the metadata from the original request will persist through modification by a `beforeSign` plugin.

The original metadata will take precedent over metadata from the new request, ensuring no plugin upstream can overwrite it. A warning will be displayed when this happens to let the developer know.

* Syntax

* Adding an appendAction and prependAction helper

* Finished testing resolve

* Fixed browser test runner

* Allow either actor + permission or permissionLevel during Session instantiation

Fixes #17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.2.x
Development

No branches or pull requests
2 participants
@aaroncox @shaqk