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

Reduce default min fill amount and validate with subgraph minFill #2319

Closed
wants to merge 4 commits into from

Conversation

psparacino
Copy link
Collaborator

Description

Adjusts default minFill to .001 so new listings will have that default minFill for purchases or retires.

Retires and Purchases use the minFill amount on the subgraph instead of hardcoded version either in the component or a constant.

one minor issue: the format from the subgraph has on decimal place. so it used to show "1" but now will show "1.0". Should that be trimmed?

Related Ticket

Closes #2317

How to Test

  1. Test a fractional retire on a current listing (should not validate as default min is 1)
  2. Create a new listing
  3. Do the same as above (new minFill amount should be .001 for new listing)
  4. Do the same above for a purchase

Copy link

vercel bot commented Feb 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
carbonmark ✅ Ready (Inspect) Visit Preview Feb 22, 2024 9:02am
carbonmark-api ✅ Ready (Inspect) Visit Preview Feb 22, 2024 9:02am
demo-integration ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2024 9:02am
klimadao-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2024 9:02am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
carbon ⬜️ Ignored (Inspect) Visit Preview Feb 22, 2024 9:02am

@0xtapi
Copy link
Collaborator

0xtapi commented Feb 23, 2024

Spotted some odd behavior. These three links all point to listings of the same credit:

https://carbonmark-git-fix-reduce-default-min-fill-amount-klimadao.vercel.app/projects/VCS-191-2008/retire/0x9e0113c4826fcdeb1a7b12ea41c2441aa77d230444836d526b744da866724d3a

https://carbonmark-git-fix-reduce-default-min-fill-amount-klimadao.vercel.app/projects/VCS-191-2008/retire/0x808ff5bb0301bbfa99fe99713ec99be6f26d6848bff9b67bdb900092046d4a3d

https://carbonmark-git-fix-reduce-default-min-fill-amount-klimadao.vercel.app/projects/VCS-191-2008/retire/0x1b6b75dd7f00ff26d71f87e4c491820dee12277e39ab1f1a7ea7c40d9e8495a5

First listing is from early December, second from mid January, third from mid February, so they are all old listings.

First and third link return errors when I try to retire fractional tonnes (as they should) and the Retire button doesn't do anything. But the second link does not show the error message, and I can click Retire. When I try to approve the transaction, this happens:

Screenshot 2024-02-23 142351

But I shouldn't even get to that point because the UI should block my attempt to retire a fractional tonne...

Also, the error message isn't quite there yet:
Screenshot 2024-02-23 141211

@psparacino
Copy link
Collaborator Author

good catch, for some reason that middle listing has a default min fill of .001 coming from the subgraph.

Not sure how that could have slipped through i.e. you updated that on the 2/15 and it somehow set the minFill to 1 where the previous listing had a minFill of .001, which shouldn't have happened to begin with
https://polygonscan.com/tx/0xb4dedb60ab32c106999000d0c09b6d9c54eceda4688825f228566fc6d88d8135#eventlog

even though that's kind of weird that shouldn't be an issue as the subgraph should pick up that and use that as the minFill for the frontend

@psparacino
Copy link
Collaborator Author

psparacino commented Feb 23, 2024

the subgraph wasn't handling minFill amounts for updating listings:

PR open in subgraph repo: KlimaDAO/klima-subgraph#119

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[form] Reduce and enforce DEFAULT_MIN_FILL_AMOUNT
4 participants