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

Update Marketplace Precompile ABI #883

Merged
merged 8 commits into from
Oct 8, 2024
Merged

Conversation

JasonTulp
Copy link
Contributor

@JasonTulp JasonTulp commented Sep 29, 2024

This PR addresses a few small inconsistencies picked up when testing the marketplace precompile.

  • Update ABI with consistent camelCase + fix compiler errors
  • Change duration to None if input is 0 (Otherwise this will start and end the listing in the same block, but not clear it)
  • Change marketplaceId to None if input is 0 (Otherwise the tx will fail due to no marketplace for id 0)

This also fixes a minor bug in the marketplace pallet that allowed listings to be created with Some(0) duration, this would result in the listing never being cleared automatically

@KarishmaBothara
Copy link
Contributor

KarishmaBothara commented Sep 30, 2024

lgtm... Thanks for updating the interface... looks great

Copy link
Contributor

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@JasonTulp JasonTulp merged commit 2c44def into main Oct 8, 2024
2 checks passed
@JasonTulp JasonTulp deleted the fix/marketplace-precompile-abi branch October 8, 2024 20:28
@zees-dev zees-dev mentioned this pull request Oct 9, 2024
4 tasks
Nick95550 pushed a commit that referenced this pull request Oct 9, 2024
* Update marketplace precompile ABI and duration + marketplace_id input params

* Fix bug in marketplace that allows for 0 duration

* fmt

* Update xrpl-bridge deps

* fmt..

* Update types in abi + update index.ts abi

* prettier
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.

3 participants