Skip to content

Conversation

@markspanbroek
Copy link
Contributor

@markspanbroek markspanbroek commented Apr 15, 2025

Uses the latest version of the Marketplace contract that utilize the Vault contract.

This includes the following changes:

  • separate reward and collateral addresses are no longer supported
  • request cancelled event is no longer emitted
  • slot state 'paid' no longer exists
  • ERC20 token contract no longer supports increaseAllowance
  • repair reward is added to slot queue, and used to calculate profitability
  • sales agent keeps track of slot queue item
  • all marketplace configuration getters are now synchronous
  • separate types for timestamps, duration, tokens, and tokens per second

And fixes the following bug:

  • slotqueue and repostore misinterpreted expiry duration for expiry timestamp

Depends on the fix in #1201
Depends on codex-storage/codex-contracts-eth#220
Depends on codex-storage/codex-contracts-eth#223

@markspanbroek markspanbroek force-pushed the marketplace-vault branch 2 times, most recently from 1e4a0ed to 73ddcce Compare April 16, 2025 12:17
@markspanbroek markspanbroek marked this pull request as ready for review April 16, 2025 13:29
On Windows the codex node did not shut down properly after this test
finished.
Collateral that needs to be provided is now
the same, regardless whether we're repairing or
not. The repair reward is added as a bonus, not
deducted as a discount.
Hardhat now has the "allowBlocksWithSameTimestamp"
property set, which made the test fail. Fixed by
explicitly increasing the timestamp in the test.
fixes bugs with slotqueue and repostore, where
expiry duration was misinterpreted as expiry
timestamp
No longer test against the Marketplace contract
directly, but only through the Market abstraction

Reason: gas estimates are often incorrect, leading
to test failures, and the same test scenarios are
already handled by testMarkets
@markspanbroek markspanbroek changed the base branch from master to fix-api-validation-test April 17, 2025 15:30
@markspanbroek markspanbroek force-pushed the fix-api-validation-test branch from 0ed671b to b3994bc Compare May 26, 2025 15:04
Base automatically changed from fix-api-validation-test to master May 26, 2025 17:44
@markspanbroek
Copy link
Contributor Author

Note to self:

# do nothing, let onCancelled callback take care of it

This is no longer true because I removed the cancelled callback, we need to exit the proving loop here.

Copy link
Contributor

@AuHau AuHau left a comment

Choose a reason for hiding this comment

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

Generally LGTM, but honestly this is the type of PR that is really hard to review.

For next time, please really break it up. From my POV, the "hard limit" of a "too big PR" is when the browser has a problem rendering the UI for the PR ... and my Brave was already having problems 😅

let market =
OnChainMarket(contract: contract, signer: signer, requestCache: requestCache)

market.configuration = ?await market.loadConfig()
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't understand this construct. I thought that the ? specifies Option for the loaded configuration, but looking at the configuration type I see that you have removed the Option there, so I am not sure what ? does in this context. I assume it is a questionable construct, but I did not really find anything about this there (or I missed it).

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.

2 participants