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

[DO NOT MERGE] feature Hooks #2217

Closed
wants to merge 15 commits into from
Closed

[DO NOT MERGE] feature Hooks #2217

wants to merge 15 commits into from

Conversation

dangell7
Copy link
Contributor

@dangell7 dangell7 commented Feb 17, 2023

High Level Overview of Change

Add Hooks Amendment Support. A full read me can be found here

Context of Change

Type of Change

This PR adds the SetHook transaction and the utility functions for the Hooks Amendment. It also makes changes to the definitions. Finally this PR is rebased off, , which is required on the Hook network.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Test Plan

I plan to write the standard tests matching a similar transaction like SignerListSet. There are 2 additional functions that needs to be written. CreateCode takes a binary string compiled from a wasm file and HookOn is a Hash256 compiled of different transaction types. Any additional files should include a test.

  • Models
  • Utils
  • Integration
  • Add credit for hook utils hook-builder-ide

@dangell7 dangell7 changed the title [WIP] feature Hooks [DO NOT MERGE] feature Hooks Feb 17, 2023
@tequdev
Copy link
Contributor

tequdev commented Feb 21, 2023

Not yet added to the documentation, no need to add an Invoke transaction?

@dangell7
Copy link
Contributor Author

Not yet added to the documentation, no need to add an Invoke transaction?

Invoke is done through HookOn unless I misunderstood you. There are a few helper functions I will add to this PR hopefully tonight or tomorrow.

@dangell7
Copy link
Contributor Author

@develoQ Here is an example of how to use the sdk.

https:/Transia-RnD/xls-playground/blob/main/ts/test/hooks.test.ts

@khancode
Copy link
Collaborator

@dangell7 Great work on this so far! One suggestion is to add a test case for adding HookParameters to non-SetHook transactions since this is now supported in testnet v3.

For example:

{
   "NetworkID":21338,
   "TransactionType":"Payment",
   "Account":"rERZinA8bgHKx1Uu7goWwzJgRFiBx2o3Xg",
   "Amount":"10000000",
   "Destination":"r3GC3EBBCMbp17pi3NjmfVPAUM7UJuyWiE",
   "DestinationTag":160,
   "HookParameters":[
      {
         "HookParameter":{
            "HookParameterName":"ABCDEF12",
            "HookParameterValue":"12345678"
         }
      }
   ],
   "Fee":"50000"
}

packages/xrpl/src/index.ts Outdated Show resolved Hide resolved
update definitions

add network id to base transaction

add set hook

update required

update definitions

make network id required
update hook

add test and validation

add estimate fee func

fix estimated fee error

add hook utils
@mvadari
Copy link
Collaborator

mvadari commented Mar 28, 2023

@dangell7 did you mean to put all these commits in this branch?

@dangell7
Copy link
Contributor Author

@dangell7 did you mean to put all these commits in this branch?

Should be cleaned up now. I'm gonna close this until we get closer to a merge.

@dangell7 dangell7 closed this Mar 28, 2023
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.

5 participants