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: make it possible to set decimalPlaces when calling startPSM #6348

Merged
merged 8 commits into from
Sep 29, 2022

Conversation

Chris-Hibbert
Copy link
Contributor

closes: #6337

Description

startPSM wasn't setting decimals when creating a new trading pair.

Also, gov-add-psm.js had the wrong keywords in its config.

Security Considerations

None.

Documentation Considerations

None

Testing Considerations

Manual

@Chris-Hibbert Chris-Hibbert added bug Something isn't working Inter-protocol Overarching Inter Protocol labels Sep 28, 2022
@Chris-Hibbert Chris-Hibbert self-assigned this Sep 28, 2022
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.

regression test pretty please

@@ -79,11 +79,19 @@ export const startPSM = async (
E(E(zoe).getInvitationIssuer()).getAmountOf(poserInvitationP),
]);

const [anchorInfo, stableInfo] = await Promise.all(
Copy link
Member

Choose a reason for hiding this comment

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

s/stable/minted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay. It starts from STABLE.symbol, but I'll rename it locally.

@@ -79,11 +79,19 @@ export const startPSM = async (
E(E(zoe).getInvitationIssuer()).getAmountOf(poserInvitationP),
]);

const [anchorInfo, stableInfo] = await Promise.all(
[anchorBrand, stable].map(b => E(b).getDisplayInfo()),
Copy link
Member

Choose a reason for hiding this comment

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

consider a note about why "display" info is reliable for this use case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have an answer for that. Can you write something that would justify it?

@dckc
Copy link
Member

dckc commented Sep 28, 2022

regression test pretty please

experience with #6187 makes that look particularly challenging. I can't find any way to diagnose why it works on my desktop and not in ci.

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I managed to test this end-to-end, locally.

@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Sep 28, 2022
@mergify mergify bot merged commit 46aa80e into master Sep 29, 2022
@mergify mergify bot deleted the 6337-decimals4startPSM branch September 29, 2022 00:44
Chris-Hibbert added a commit that referenced this pull request Sep 29, 2022
* fix: make it possible to set decimalPlaces when calling startPSM

* fix: makeRatio wants bigInt

* chore: type cleanup and provisioning permit

* chore: some tests don't have defaults for anchor decimalPlaces

* chore: rename stable token in startPSM to `minted`

* fix(agoric-cli): look up correct brand for PSM trade

* chore: ignore missing type for pair

Co-authored-by: Dan Connolly <connolly@agoric.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Chris-Hibbert added a commit that referenced this pull request Sep 29, 2022
…) (#6357)

* fix: make it possible to set decimalPlaces when calling startPSM

* fix: makeRatio wants bigInt

* chore: type cleanup and provisioning permit

* chore: some tests don't have defaults for anchor decimalPlaces

* chore: rename stable token in startPSM to `minted`

* fix(agoric-cli): look up correct brand for PSM trade

* chore: ignore missing type for pair

Co-authored-by: Dan Connolly <connolly@agoric.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Co-authored-by: Dan Connolly <connolly@agoric.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge bug Something isn't working Inter-protocol Overarching Inter Protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adding USDT with 18 decimals gets 1:1 anchorPerMinted despite 6 decimals for IST
3 participants