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

fix(agops): fix continuing id lookup in oracle setPrice #8457

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

dckc
Copy link
Member

@dckc dckc commented Oct 14, 2023

needs instance based on --pair

closes: #8281
refs: #XXXX

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

  • IOU a test

Upgrade Considerations

@dckc
Copy link
Member Author

dckc commented Oct 14, 2023

cc @0xpatrickdev

@dckc
Copy link
Member Author

dckc commented Nov 29, 2023

I looked into doing a test. Threading IO capabilities explicitly looks like more trouble than it's worth (#2160).

How about landing this as is?

@dckc dckc marked this pull request as ready for review November 29, 2023 21:57
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

Threading IO capabilities explicitly looks like more trouble than it's worth.

It would be fine to use ambient IO capabilities for a test.

I looked into doing a test.

A test would be nice but since this is an internal tool, if you've tested it manually that suffices for me.

@dckc
Copy link
Member Author

dckc commented Nov 29, 2023

It would be fine to use ambient IO capabilities for a test.

Hm. It would go out to the network and such then, right? I would need to run a blockchain node or something.
The idea was to mock what it would find out in the world.

Alternatively, I could factor out the part that I fixed and unit test that, but again, it's kind of a pain.

@dckc dckc added the automerge:rebase Automatically rebase updates, then merge label Nov 29, 2023
@mergify mergify bot merged commit 3238732 into master Nov 30, 2023
68 checks passed
@mergify mergify bot deleted the dc-setPrice-mcs branch November 30, 2023 00:09
@turadg
Copy link
Member

turadg commented Nov 30, 2023

The idea was to mock what it would find out in the world.

Yeah, I was assuming we could mock module imports. That's built into Jest. In Ava it would take something like https://github.com/iambumblehead/esmock

@dckc
Copy link
Member Author

dckc commented Nov 30, 2023

... mock module imports ...

Yeah, that's another approach. I didn't look into it. Does it make a test seem more cost-effective?

@turadg
Copy link
Member

turadg commented Nov 30, 2023

More but not enough imo. Also using esmock-loader precludes using tsx-loader.

mhofman pushed a commit that referenced this pull request Dec 6, 2023
fix(agops): fix continuing id lookup in oracle setPrice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use setPrice in CLI to reliably change ATOM and stATOM price (for testing)
2 participants