decentraland / signatures-server / 30306353503

92%

main: 92%

Build Type

Pull #487

github

Committed by LautaroPetaccio

Commit Message

fix: stop cancelling listings whose asset the rentals contract holds

the unauthenticated PATCH /v1/rentals-listings/:id cancelled any open listing whose asset was
not owned by the lessor, without excluding the case its own comment described: the rentals
contract holding the asset on the lessor's behalf. that is the supported flow of re-listing
after a rent ended without claiming the asset back, so anyone could enumerate listing ids
through the public GET and cancel them one by one. updateMetadata had the same defect written
differently, comparing the nft owner against the on chain lessor, a comparison that never
matched, so its guard never returned early either. both paths now share one check.

other fixes in the same area:

- the contract index cancellation wrapped a bound parameter in double quotes, which postgres
  reads as an identifier, so the statement failed, the whole batch rolled back and the job
  never advanced its cursor again. listings invalidated on chain stayed open.
- the rental contract address was stored as the caller sent it. it drives the index based
  cancellations, so repairing the statement above without validating it would have turned an
  inert bug into a bypass. it is now checked against the rentals contract of the chain.
- the cancellation issued during a refresh was neither awaited nor ordered against the
  executed update it raced. the service runs with unhandled-rejections=strict, so a rejection
  there took the process down.
- the lookup by signature in updateRentalsListings relied on and binding tighter than or, so
  neither branch joined every table and period_chosen could point at another listing's period.
- updateMetadata stored the indexer timestamp with the conversion inverted, writing dates near
  1970. that made the refresh treat the metadata as stale on every call.
- cancelRentalsListings computed a cursor it never passed to the query, so a window with a
  full page of index updates looped forever holding a tra... (continued)

Pull Request Pull Request #487: fix: stop cancelling listings whose asset the rentals contract holds

Coverage Stats

339 of 366 branches covered (92.62%)

Branch coverage included in aggregate %.

53 of 55 new or added lines in 7 files covered. (96.36%)

2 existing lines in 1 file now uncovered.

676 of 742 relevant lines covered (91.11%)

44.89 hits per line