feat(PE-6656) - return initiated dutch auctions #92

dtfiedler · 2024-10-09T18:37:51Z

Summary:
Introduces and Auction object class that is used to store auctions on the NameRegistry. We should look towards using more of these classes to leverage some better static analysis and type enforcement on functions. The class is responsible for computing prices based on the initialization details (baseFee, demandFactor, etc.). Auctions are stored as instances of this class, and returned as simple table (excluding function) so clients can compute prices themselves.

This initial implementation includes handler support for ANTs returning permabuy name registrations. In a separate PR we'll introduce Lease-Expiration-Intiated-Auctions where a name will go in to auction after the grace period has ended.

Checklist:

business logic (and docs) to create and store auctions in state with pricing information
handler for Release-Name that allows integration with ANTs
handler for Auction-Info to get auction information of a given name
handler for Auction-Bid to submit bid to acquire name in auction and update arns registry
unit tests with >80% coverage
integration tests for all handlers
update ANT source code to support Release-Name handler that forwards to this process
- PR: feat(io): add handler that forwards a Release-Name notice to the pr… ar-io-ant-process#23
update ar-io-sdk to include auction types and APIs for auctions
- PR: feat(auctions): add auctions api to IO classes ar-io-sdk#235
add event data for auction creation and auction bids
- WILL DO IN A SEPARATE PR ONCE E2E TESTING IN DEVNET HAS BEEN VALIDATED

TODOs:

storing prices in auction state impacts the handler response (Data ends up being an empty table - not sure if this is a bug in AO or we are hitting a memory limit). it also costs ~600KiB to store those values in state, which could bloat our memory when several auctions are in process. we could consider just storing the settings of the auction pricing in state, and use that to calculate prices when necessary (still have to create the table, but response doesn't need to contain it). need to think more about this and what is necessary for clients to be able to showing pricing charts.
- we've modified to store the auction settings on the auction object, and then clients will compute the price with various parameters

Here is the sequence diagram for a permabuy initiated auction

codecov-commenter · 2024-10-09T18:38:41Z

Codecov Report

Attention: Patch coverage is 99.15966% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.07%. Comparing base (9e6395b) to head (aec2ee4).

Files with missing lines	Patch %	Lines
src/arns.lua	98.68%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop      #92      +/-   ##
===========================================
+ Coverage    89.50%   90.07%   +0.57%     
===========================================
  Files            8        9       +1     
  Lines         1724     1834     +110     
===========================================
+ Hits          1543     1652     +109     
- Misses         181      182       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… with tests

This is so we can discern where tokens go when auctions (and other events) happen.

…date tests

arielmelendez · 2024-10-14T21:12:28Z

General request to add events for this flow when the PR is approaching readiness for review.

We will add the settings to the auction, including decayRate and scalingExponent so clients can dynamically compute prices.

spec/arns_spec.lua

atticusofsparta · 2024-10-22T18:47:14Z

src/arns.lua

@@ -278,7 +279,7 @@ function arns.assertValidBuyRecord(name, years, purchaseType, processId)
 	assert(name:match("^%w") and name:match("%w$") and name:match("^[%w-]+$"), "Name pattern is invalid.")


should we have a util for this?

atticusofsparta · 2024-10-22T18:49:05Z

src/arns.lua

+--- @param name string The name of the auction
+--- @param timestamp number The timestamp to start the auction
+--- @param initiator string The address of the initiator of the auction
+--- @return Auction|nil The auction instance


atticusofsparta · 2024-10-22T18:52:09Z

src/arns.lua

+function arns.getAuction(name)
+	return NameRegistry.auctions[name]
+end
+
+function arns.getAuctions()
+	return NameRegistry.auctions or {}
+end


the or {} implies auctions could be undefined which would indicate that NameRegistry.auctions[name] could attempt to index a nil value

it can be, if we don't update the state manually

if we don't update the state manually
Is that the plan?

arielmelendez · 2024-10-23T16:32:16Z

spec/arns_spec.lua

+			end)
+		end)
+
+		describe("getAuction", function()


Could add a test for a missing auction

arielmelendez · 2024-10-23T17:03:33Z

src/arns.lua

-	then
+	local record = arns.getRecord(name)
+	local isPermabuy = record ~= nil and record.type == "permabuy"
+	local isActiveLease = record ~= nil and (record.endTimestamp or 0) + constants.gracePeriodMs > timestamp


Nit: asserting the existence of the endTimestamp might be a little more robust.

arielmelendez · 2024-10-23T17:06:34Z

src/arns.lua

@@ -48,18 +50,17 @@ function arns.buyRecord(name, purchaseType, years, from, timestamp, processId)
 		type = purchaseType,
 		undernameLimit = constants.DEFAULT_UNDERNAME_COUNT,
 		purchasePrice = totalRegistrationFee,
+		endTimestamp = purchaseType == "lease" and timestamp + constants.oneYearMs * years or nil,


Lua's control flow in these scenarios freaks me out.

(I realize it's just a ternary but I wish they'd just use the same control characters as everyone else)

arielmelendez · 2024-10-23T17:09:56Z

src/arns.lua

+-- AUCTIONS
+
+--- Creates an auction for a given name
+--- @param name string The name of the auction


Nit: "the name being auctioned" otherwise this reads like it's an auction title

arielmelendez · 2024-10-23T17:19:25Z

src/arns.lua

+	end
+
+	-- assert the bid is between auction start and end timestamps
+	if timestamp < auction.startTimestamp or timestamp > auction.endTimestamp then


Should endTimestamp be inclusive?

arielmelendez · 2024-10-23T17:20:42Z

src/arns.lua

+
+	-- assert the bid is between auction start and end timestamps
+	if timestamp < auction.startTimestamp or timestamp > auction.endTimestamp then
+		-- TODO: we should likely clean up the auction if it is outside of the time range


Prune should catch that IFF the endTimestamp is exclusive (i.e. prune would end an auction AT the end timestamp if they overlapped).

arielmelendez · 2024-10-23T17:22:17Z

src/arns.lua

+		error("Bid timestamp is outside of auction start and end timestamps")
+	end
+	local requiredBid = auction:getPriceForAuctionAtTimestamp(timestamp, type, years)
+	local requiredOrBidAmount = bidAmount or requiredBid


What's the objective here? Let callers NOT supply a bidAmount?

arielmelendez · 2024-10-23T17:34:05Z

src/arns.lua

+		processId = processId,
+		startTimestamp = timestamp,
+		endTimestamp = type == "lease" and timestamp + constants.oneYearMs * years or nil,
+		undernameLimit = constants.DEFAULT_UNDERNAME_COUNT,


If the record had previously increased its undername limit, is that lost in transition here?

arielmelendez · 2024-10-23T17:37:04Z

src/arns.lua

+	arns.removeAuction(name)
+	arns.addRecord(name, record)
+	-- make sure we tally name purchase given, even though only half goes to protocol
+	-- TODO: DO WE WANT TO TALLY THE ENTIRE AMOUNT OR JUST THE REWARD FOR THE PROTOCOL?


Entire amount IMO

arielmelendez · 2024-10-23T17:41:35Z

src/arns.lua

+function arns.pruneAuctions(currentTimestamp)
+	local prunedAuctions = {}
+	for name, auction in pairs(arns.getAuctions()) do
+		if auction.endTimestamp <= currentTimestamp then


Note here to make sure that end timestamp is handled consistently in various functions (inclusively or exclusively during bidding, pruning, etc.)

arielmelendez · 2024-10-23T17:46:45Z

src/auctions.lua

+-- Default Auction Settings
+AuctionSettings = {
+	durationMs = 60 * 1000 * 60 * 24 * 14, -- 14 days in milliseconds
+	decayRate = 0.000000000016847809193121693, -- 0.02037911 / durationMs


Note that this is 27 digits after the decimal so keep that in mind if passing across process boundaries.

arielmelendez · 2024-10-23T17:53:43Z

src/auctions.lua

+		},
+	}
+	setmetatable(auction, self)
+	self.__index = self


Seems there's some nuances to setting up the class this way (in the constructor) vs. separately. Will share chatGPT thread with you separately about it.

arielmelendez · 2024-10-23T17:59:15Z

src/auctions.lua

+--- Computes the prices for the auction.
+--- @param type string The type of auction
+--- @param years number The number of years for calculation
+--- @param intervalMs number The interval in milliseconds, must be at least 15 minutes


Note to double check where the 15 minutes is enforced.

arielmelendez · 2024-10-23T18:12:42Z

src/main.lua

@@ -676,7 +679,14 @@ addEventingHandler(ActionMap.BuyRecord, utils.hasMatchingTag("Action", ActionMap
 	ao.send({
 		Target = msg.From,
 		Tags = { Action = "Buy-Record-Notice", Name = msg.Tags.Name },
-		Data = json.encode(record),
+		Data = json.encode({


What was the material difference here?

arielmelendez · 2024-10-23T18:18:21Z

src/main.lua

+	local name = msg.Tags.Name
+	local processId = msg.From
+	local record = arns.getRecord(name)
+	local initiator = msg.Tags.Initiator


Can we be getting this from a Forwarded-For tag or similar that's provided by the MU during message forwarding?

arielmelendez · 2024-10-23T18:25:08Z

src/main.lua

+	end
+
+	-- we should be able to create the auction here
+	local status, auctionOrError = pcall(arns.createAuction, name, tonumber(msg.Timestamp), initiator)


It's pretty much free to do these as eventingPcalls now since we wrapped all the handlers.

arielmelendez · 2024-10-23T18:25:44Z

src/main.lua

+		return
+	end
+
+	local auction = {


Why is this step necessary?

arielmelendez · 2024-10-23T18:27:04Z

src/main.lua

+	local timestamp = tonumber(msg.Tags.Timestamp or msg.Timestamp)
+	local type = msg.Tags.Type or "permabuy"
+	local years = msg.Tags.Years and tonumber(msg.Tags.Years) or nil
+	local intervalMs = msg.Tags["Price-Interval-Ms"] and tonumber(msg.Tags["Price-Interval-Ms"]) or 15 * 60 * 1000 -- 15 minute intervals by default


We should enforce a minimum interval.

arielmelendez · 2024-10-23T18:28:08Z

src/main.lua

+	})
+end)
+
+addEventingHandler("auctionBid", utils.hasMatchingTag("Action", ActionMap.AuctionBid), function(msg)


Why did we move away from Buy-Record as the one true path for acquiring names?

dtfiedler changed the title ~~Pe 6656 auctions~~ PE-6656 - return initiated dutch auctions Oct 9, 2024

dtfiedler added 8 commits October 9, 2024 17:23

feat(auctions): initial functions and global state for auctions

c9d531a

chore(docs): add sequence diagram for dutch auction

b7b1b49

chore(test): update unit tests to create auction

897b3e5

chore(test): add test validating bid at specific timestamp

c0336e4

fix(main): add initial handlers for Release-Name and Auction-Info…

5b2ccec

… with tests

chore(format): run prettier on all files

20e3bf6

chore(test): add more unit tests

7a6b91e

chore(test): standardize PROCESS_OWNER and PROCESS_ID

6749531

This is so we can discern where tokens go when auctions (and other events) happen.

dtfiedler force-pushed the PE-6656-auctions branch from 80b9792 to 6749531 Compare October 9, 2024 23:23

dtfiedler added 4 commits October 9, 2024 18:28

chore(test): add back test for event data

d368179

chore(test): add back gar tests

2e59133

fix(auctions): restrict releasing names to only permabought names, up…

ababf1b

…date tests

chore(docs): cleanup outdated or no longer applicable TODO comments

6975c52

dtfiedler mentioned this pull request Oct 14, 2024

feat(io): add handler that forwards a Release-Name notice to the pr… ar-io/ar-io-ant-process#23

Merged

dtfiedler added 6 commits October 21, 2024 09:09

fix(auctions): modify pricing logic for auctions

e923629

We will add the settings to the auction, including decayRate and scalingExponent so clients can dynamically compute prices.

chore(test): fix integration tests

de34475

chore(arns): update auction pricing logic and decay rate, add tests

2d46ef3

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

c0ab614

chore(test): fix tests after merge

8df6333

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

a4b92a3

dtfiedler marked this pull request as ready for review October 21, 2024 16:26

dtfiedler added 4 commits October 21, 2024 15:07

chore(arns): use auction class instead of simple table object

e210904

fix(auctions): update integration handlers and tests

3fece52

chore(test): more test updates

f684dd2

chore(type): tweak auction type

f242629

dtfiedler changed the title ~~PE-6656 - return initiated dutch auctions~~ feat(PE-6656) - return initiated dutch auctions Oct 21, 2024

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

d3cf1c9

atticusofsparta reviewed Oct 22, 2024

View reviewed changes

spec/arns_spec.lua Show resolved Hide resolved

atticusofsparta reviewed Oct 22, 2024

View reviewed changes

dtfiedler added 2 commits October 22, 2024 14:20

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

4dc0b06

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

aec2ee4

dtfiedler force-pushed the PE-6656-auctions branch from 81070a3 to aec2ee4 Compare October 23, 2024 14:50

dtfiedler added 2 commits October 23, 2024 10:08

chore(test): fix integration test with updated precision

2c6572a

Merge remote-tracking branch 'origin/develop' into PE-6656-auctions

c48a9d6

dtfiedler merged commit 6d76e21 into develop Oct 23, 2024

dtfiedler deleted the PE-6656-auctions branch October 23, 2024 15:25

arielmelendez reviewed Oct 23, 2024

View reviewed changes

spec/arns_spec.lua

end)

end)

describe("getAuction", function()

Copy link

Contributor

arielmelendez Oct 23, 2024

Choose a reason for hiding this comment

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

Could add a test for a missing auction

arielmelendez reviewed Oct 23, 2024

View reviewed changes

src/main.lua

return

end

local auction = {

Copy link

Contributor

arielmelendez Oct 23, 2024

Choose a reason for hiding this comment

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

Why is this step necessary?

arielmelendez reviewed Oct 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(PE-6656) - return initiated dutch auctions #92

feat(PE-6656) - return initiated dutch auctions #92

dtfiedler commented Oct 9, 2024 •

edited

Loading

codecov-commenter commented Oct 9, 2024 •

edited

Loading

arielmelendez commented Oct 14, 2024

atticusofsparta Oct 22, 2024

dtfiedler Oct 22, 2024

atticusofsparta Oct 22, 2024

atticusofsparta Oct 22, 2024

dtfiedler Oct 22, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

arielmelendez Oct 23, 2024

		@@ -278,7 +279,7 @@ function arns.assertValidBuyRecord(name, years, purchaseType, processId)
		assert(name:match("^%w") and name:match("%w$") and name:match("^[%w-]+$"), "Name pattern is invalid.")

feat(PE-6656) - return initiated dutch auctions #92

feat(PE-6656) - return initiated dutch auctions #92

Conversation

dtfiedler commented Oct 9, 2024 • edited Loading

codecov-commenter commented Oct 9, 2024 • edited Loading

Codecov Report

arielmelendez commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtfiedler commented Oct 9, 2024 •

edited

Loading

codecov-commenter commented Oct 9, 2024 •

edited

Loading