feat: Initial implementation of auction dApp #2265

mous1985 · 2024-06-03T10:12:23Z

Initial Implementation of dAuction

Purpose

This pull request introduces the initial version (v0) of the auction dApp. The dApp allows users to create and manage auctions. It is designed to interact with an existing NFT realm to handle the state and ownership of NFTs, with future plans to integrate NFTs directly into the auction struct and automate transfers upon auction completion.
Main Features

Auction Creation: Users can create new auctions specifying the title, description, start time, end time, and minimum price.
Bidding: Users can place bids on active auctions. Each bid must be higher than the current highest bid.
Auction End: When the auction ends, the highest bidder wins.
State Management: The auction state is managed by the realm, ensuring persistence and consistency across transactions.

How It Works

p/demo/auction:
The auction package handles the core logic for creating and * managing auctions.
It defines the Auction and Bid structs and provides methods for creating auctions, adding bids, and ending auctions.
Future plans include integrating NFTs into the Auction struct to directly manage NFT auctions.
r/demo/auction:
The auction realm maintains the state of all auctions using an AVL tree.
It uses the auction package to handle auction operations and interacts with the NFT realm to manage NFT state and transfers.
The realm exposes methods for creating auctions, placing bids, and rendering the current state.
Testing:
The auction_test.gno file contains test cases that simulate auction creation, bidding, and ending.
Tests ensure that the core functionalities work as expected, including state changes and interactions with the NFT realm.

Future Enhancements

Integrate NFTs: Future updates will include adding an nft.TokenID field to the Auction struct and enhancing the logic to support direct NFT auctions.
Deposits and Automatic Transfers:

NFT Deposit: The NFT will be deposited into the realm when the auction starts.
Bid Deposit: Bids will be deposited into the realm.
Automatic Transfer: Upon auction completion, the NFT will be automatically transferred to the highest bidder, and the highest bid amount will be transferred to the seller.

Additional Features: Plan to add more features such as auction cancellation, bid retraction, and more detailed auction statistics .

PS: This pull request lays the foundation for the auction dApp, and future enhancements can build on this initial implementation to add more features and improve functionality. Feel free to contribute and share any ideas :) .
@moul @leohhhn

Contributors' checklist...

Added new tests, or not needed, or not feasible
Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
Updated the official documentation or not needed
No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
Added references to related issues and PRs
Provided any useful hints for running manual tests
Added new benchmarks to generated graphs, if any. More info here.

deelawn

Auctions are a common thing in other web3 ecosystems, so thanks for adding this -- it's a good start.

What is the intended way to have users interact with auctions? I see that the realm returns an instance of the auction object to the caller. This may not be desirable because the person receiving the auction object could call the AddBid method, circumventing the the checks inside the the realm's AddBid function.

It seems to me like the realm would be the container for all auction objects and the auction specific logic could live within the package, so the error checking logic in the realm's AddBid function could be moved to the package, unless the intention is to give realms using the package full control over how these checks should look, though some of them seem pretty universal.

Also, I left a comment regarding the auction structs' fields being unexported. Exporting these fields would remove the need for all of the accessor methods, as well as let the realms using the package decide whether or not they want to let auction owners adjust auction details after they've been created. I could be persuaded either way; just something to consider.

deelawn · 2024-06-04T17:06:55Z

examples/gno.land/p/demo/auction/auction.gno

+
+// Main struct
+type Auction struct {
+ title string


Is there any reason why the fields for these structs can't be exported so we can remove all of the getter methods? If auctions are made to be self contained within the realm itself (don't return any references to auctions), then it should be okay.

I'm really sorry for the long delay in replying. 6936b8

examples/gno.land/p/demo/auction/auction.gno

deelawn · 2024-06-04T17:11:42Z

examples/gno.land/p/demo/auction/auction.gno

+}
+
+// NewBid creates a new Bid instance.
+func NewBid(bidder std.Address, amount uint64) *Bid {


I think it's better to get rid of this function -- not calling it will save gas and it's only used in one spot

I've removed the NewBid() function and updated the AddBid() and the IsOwnedBy() functions 5e47aa9.

deelawn · 2024-06-04T17:13:15Z

examples/gno.land/p/demo/auction/auction.gno

+ panic("Auction cannot end before the end time")
+ }
+ if a.bids.Size() == 0 {
+ panic("No bids placed")


Is this correct? If the end time has passed for an auction then the realm prevents adding a bid, but this prevents marking the auction as ended, so the auction can never end.

I replaced panic by ufmt.Errorf which handles other error cases dd5b4911

deelawn · 2024-06-04T17:20:28Z

examples/gno.land/p/demo/auction/auction.gno

+ begin time.Time
+ end time.Time
+ price uint64
+ bids *avl.Tree // key: std.Address, value: Bid


I think this comment is wrong; the key being used when adding new bids is a integer string.

Even so, why use an avl tree to represent bids instead of something like a slice? The bid struct contains the bidder's address already, so no need to use it as a key. Using a integer as a key is kind of like using a slice where the integer is the key by default.

deelawn · 2024-06-04T17:53:05Z

examples/gno.land/r/demo/auction/auction.gno

+}
+
+// Mockable function to get current time
+func now() time.Time {


It would be better to keep testing logic in the testing file. There is a pending PR that will allow you to change the timestamp from your test code; it would be good to use that when it is merged #569

Agreed, please drop currentTime

deelawn · 2024-06-04T17:56:33Z

examples/gno.land/r/demo/auction/auction.gno

+}
+
+// renderHomepage renders the homepage of the realm
+func renderHomepage() string {


Rendering all auctions with all bids is a lot for one page. Consider having the homepage render links to each auction that, when rendered, display all the auction bids.

i updated the auction realm and i imported the mux package and added render pages for each auction state 9d634721

mous1985 · 2024-06-05T14:40:12Z

Hi @deelawn,

Thank you for the detailed feedback! I appreciate your insights and suggestions.

I understand the possibility of users bypassing the controls inside the realm's AddBid function by calling methods directly on the auction object. To address this, I propose the following changes:

The realm will manage all interactions with auction objects. Instead of returning the auction instance, the realm will provide functions to interact with the auction, such as PlaceBid, EndAuction, etc.
Users will not receive direct references to the auction objects. Instead, they will interact with the auctions through the realm's interface, ensuring that all necessary checks are enforced.
Only the owner of the auction will be allowed to modify certain fields, such as the title and description. This ensures that unauthorized users cannot change the auction details.
it's true that rendering all the auctions with all the offers is a lot for one page. I will adjust it to render links to each auction, displaying all bids when the specific auction is viewed.

I am currently working on implementing these changes and will push an update shortly. Thank you for the constructive feedback and helping :)

Co-authored-by: deelawn <[email protected]>

mous1985

Hi @deelawn,

Thanks again for your insights,I wanted to let you know that I plan to integrate accesscontrol and timelock to give the bidders time to withdraw if there is something fishy functionalities into the logic of the auction.
Please let me know if you have any further suggestions or comments.

zivkovicmilos · 2024-08-15T11:24:05Z

@mous1985
Can you please check why the CI is failing? 🙏

zivkovicmilos

Looks good 💯

The tests could use a bit more work, I think proptests would fit in nicely here.

Please see @deelawn's comments -- I've also left a couple 🙏

zivkovicmilos · 2024-08-15T11:28:43Z

examples/gno.land/p/demo/auction/auction.gno

+ "gno.land/p/demo/avl"
+)
+
+// Main struct


There is no needs for comments like these :)

I'm sorry for the delay in replying 🙏.I deleted the comment 😅 ,and replaced avlTree with a simple slice 6936b8

zivkovicmilos · 2024-08-15T11:30:40Z

examples/gno.land/p/demo/auction/auction.gno

+ title string,
+ owner std.Address,
+ description string,
+ begin time.Time,


What do you think about having begin actually be time.Now?
This way you can decrease the param. It's only assuming you don't have auctions that start in the future, given that you emit an event when this constructor exits

end can be time.Duration if this is your preference.

I've updated the package with three auction states: “upcoming”, “ongoing”, and “closed”. That was my idea, so I could have auctions starting in the future.

examples/gno.land/p/demo/auction/auction.gno

zivkovicmilos · 2024-08-15T11:31:58Z

examples/gno.land/p/demo/auction/auction.gno

+// GetBids returns an array of all the bids made in the auction.
+func (a Auction) GetBids() []*Bid {
+ bids := make([]*Bid, 0, a.bids.Size())
+ a.bids.Iterate("", "", func(key string, value interface{}) bool {


What do you think about adding some kind of pagination here, to circumvent an execution attack if there are many bids?

zivkovicmilos · 2024-08-15T11:32:27Z

examples/gno.land/p/demo/auction/auction.gno

+}
+
+// IsOwner checks if the given address is the owner of the auction.
+func (a Auction) IsOwnedBy(address std.Address) bool {


You can use Ownable

zivkovicmilos · 2024-08-15T11:39:10Z

examples/gno.land/r/demo/auction/auction.gno

+ if !ok {
+ panic("auction does not exist")
+ }
+ auction := auc.(*auctionp.Auction)


I think you can delegate these checks to the package AddBid instead

zivkovicmilos · 2024-08-15T11:40:28Z

examples/gno.land/r/demo/auction/auction.gno

+// renderHomepage renders the homepage of the realm
+func renderHomepage() string {
+ var b bytes.Buffer
+ b.WriteString("<h1><center>Auctions</center></h1>\n\n")


Why not a simple #?

zivkovicmilos · 2024-08-15T11:41:15Z

examples/gno.land/r/demo/auction/auction.gno

+ return b.String()
+}
+
+// Helper function to set the mock current time


You shouldn't need these

zivkovicmilos · 2024-08-15T11:41:37Z

examples/gno.land/r/demo/auction/auction_test.gno

+ begin := now().Add(1 * time.Second).Unix() // Auction begins in 1 second
+ end := now().Add(24 * time.Hour).Unix() // Auction ends in 24 hours
+ auc := NewAuction("Test Auction", "A simple test auction", begin, end, 100)
+ if auc == nil {


You can use uassert here

zivkovicmilos · 2024-08-15T11:41:58Z

examples/gno.land/r/demo/auction/auction_test.gno

+ setcurrentTime(now().Add(2 * time.Second))
+
+ // Place bids by different users
+ user1 := std.Address("user1")


You should use testutils for generating an address, not casting it directly like this

codecov · 2024-08-20T16:06:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.20%. Comparing base (04d5239) to head (9ebe6b9).
Report is 13 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2265      +/-   ##
==========================================
- Coverage   60.23%   60.20%   -0.03%     
==========================================
  Files         562      562              
  Lines       75091    75038      -53     
==========================================
- Hits        45230    45178      -52     
- Misses      26481    26483       +2     
+ Partials     3380     3377       -3

Flag	Coverage Δ
contribs/gnodev	`61.40% <ø> (ø)`
contribs/gnofaucet	`14.46% <ø> (-0.86%)`	⬇️
gnovm	`64.33% <ø> (ø)`
misc/genstd	`80.54% <ø> (ø)`
misc/logos	`19.88% <ø> (ø)`
tm2	`61.95% <ø> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

gfanton · 2024-08-26T07:41:09Z

@mous1985, What is the status on this? Can you check and address the comments?

replace the auction liste avlTree by slice

update AddBid update isOwnedBy

mous1985 added 8 commits May 28, 2024 19:14

mvp

6016549

package realm

a5612ca

add emit event

d1f20c6

wal

11f328c

add event

efa50bf

element in lowercase

96b652b

r

cc559a4

pull request v0

1b2221c

github-actions bot assigned mous1985 Jun 3, 2024

github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jun 3, 2024

mous1985 changed the title ~~dAuction~~ Auction dapp Jun 3, 2024

mous1985 changed the title ~~Auction dapp~~ feat: Initial implementation of auction dApp Jun 3, 2024

mous1985 and others added 5 commits June 3, 2024 14:02

fix error std.Emit()

16ad976

fix error std.Emit() n1

e143032

Merge branch 'master' into auction

581dbdf

fix error too many argument

9ca1253

Merge branch 'auction' of https://github.com/mous1985/gno into auction

2b6c1f4

mous1985 marked this pull request as ready for review June 4, 2024 12:32

mous1985 requested review from a team as code owners June 4, 2024 12:32

mous1985 requested review from gfanton and zivkovicmilos and removed request for a team June 4, 2024 12:32

deelawn reviewed Jun 4, 2024

View reviewed changes

Apply suggestions from code review

a291aa1

Co-authored-by: deelawn <[email protected]>

mous1985 commented Jun 8, 2024

View reviewed changes

michelleellen mentioned this pull request Jun 12, 2024

Minutes: Public Dev Calls [every Wednesday] gnolang/meetings#37

Open

zivkovicmilos approved these changes Aug 15, 2024

View reviewed changes

Merge branch 'master' into auction

e384536

mous1985 added 10 commits August 28, 2024 14:47

update struct fields and delete the getter methods

6936b8b

replace the auction liste avlTree by slice

delete newBid

5e47aa9

update AddBid update isOwnedBy

update EndAuction with other error cases

dd5b491

update auction_test.gno

0d94343

update auction.gno realm

9d63472

update auction test realm

9a8ec0a

use std.Coin instead of price

af0ae2e

change IsOwnedBy with ownable

5b7ecaf

ownable

45a2523

update auciton_test.gno

9ebe6b9

mous1985 force-pushed the auction branch from d2595d2 to 9ebe6b9 Compare September 3, 2024 10:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Initial implementation of auction dApp #2265

feat: Initial implementation of auction dApp #2265

mous1985 commented Jun 3, 2024

deelawn left a comment

deelawn Jun 4, 2024

mous1985 Aug 28, 2024

deelawn Jun 4, 2024

mous1985 Aug 28, 2024

deelawn Jun 4, 2024

mous1985 Aug 28, 2024

deelawn Jun 4, 2024

deelawn Jun 4, 2024

zivkovicmilos Aug 15, 2024

deelawn Jun 4, 2024

mous1985 Aug 28, 2024

mous1985 commented Jun 5, 2024

mous1985 left a comment •

edited

Loading

zivkovicmilos commented Aug 15, 2024

zivkovicmilos left a comment

zivkovicmilos Aug 15, 2024

mous1985 Aug 28, 2024

zivkovicmilos Aug 15, 2024

mous1985 Aug 28, 2024

zivkovicmilos Aug 15, 2024

zivkovicmilos Aug 15, 2024

mous1985 Aug 30, 2024

zivkovicmilos Aug 15, 2024

zivkovicmilos Aug 15, 2024

zivkovicmilos Aug 15, 2024

zivkovicmilos Aug 15, 2024

zivkovicmilos Aug 15, 2024

codecov bot commented Aug 20, 2024 •

edited

Loading

gfanton commented Aug 26, 2024

feat: Initial implementation of auction dApp #2265

Are you sure you want to change the base?

feat: Initial implementation of auction dApp #2265

Conversation

mous1985 commented Jun 3, 2024

Initial Implementation of dAuction

Purpose

How It Works

Future Enhancements

deelawn left a 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

mous1985 commented Jun 5, 2024

mous1985 left a comment • edited Loading

Choose a reason for hiding this comment

zivkovicmilos commented Aug 15, 2024

zivkovicmilos left a 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

codecov bot commented Aug 20, 2024 • edited Loading

Codecov Report

gfanton commented Aug 26, 2024

mous1985 left a comment •

edited

Loading

codecov bot commented Aug 20, 2024 •

edited

Loading