-
Notifications
You must be signed in to change notification settings - Fork 523
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
Aliter: Implement DeliverMax alias in Payment transactions, through autofill method #2689
Conversation
Do you guys have any ideas for complying with lint errors? For instance:
|
@mvadari Do you have any workarounds for the lint errors? type-erasure does not seem to sit well with the linter. |
Feel free to disable the linter error for that line. |
thanks, I've fixed the linter errors and warnings. I didn't realize that I could ignore the errors and get away with it haha I think it's safe to ignore the linter errors because, a malformed input is sure to through an error in signing or serialization steps. |
sorry about the forced updates, I had messed up the git history |
We should also add integ tests to verify it works for both API versions under |
Integration tests are set up against rippledv2.2.0-b3 in the current codebase. I tried to execute the integration tests against v1.12.0 (this rippled version contains API v1), but I'm running into some config errors |
…ent integration tests
…ds -- it does not pertain to rippled code include type annotation in the payment integration test cases
Co-authored-by: Omar Khan <[email protected]>
Co-authored-by: Omar Khan <[email protected]>
Co-authored-by: Omar Khan <[email protected]>
Co-authored-by: Omar Khan <[email protected]>
paymentTx.DeliverMax = '6789' | ||
paymentTx.Amount = '1234' | ||
|
||
await assertRejects(testContext.client.autofill(paymentTx), ValidationError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justinr1234 here is the test case where the ValidationError
is caught as expected -- there is not websocket connection to a rippled instance, but it traverses the new code path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not an integration test, hence it does not involve any codepaths through connection.ts
or other websocket related functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
High Level Overview of Change
This is an alternative solution for the DeliverMax alias, inspired from this discussion: #2684 (comment)
Note: Only one of these two PRs must be merged.
Both the PRs seek to achieve the same objectives, albeit through different means.
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan
Added unit tests