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

feat: Initial implementation of auction dApp #2265

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions examples/gno.land/p/demo/auction/auction.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package auction

import (
"std"
"strconv"
"time"

"gno.land/p/demo/avl"
)

// Main struct
Copy link
Member

Choose a reason for hiding this comment

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

There is no needs for comments like these :)

Copy link
Author

Choose a reason for hiding this comment

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

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

type Auction struct {
title string
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

owner std.Address
description string
begin time.Time
end time.Time
price uint64
bids *avl.Tree // key: std.Address, value: Bid
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}

type Bid struct {
bidder std.Address
amount uint64
}

type EventEnd struct {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is ever used, you can drop it

winner std.Address
amount uint64
}

func NewAuction(
title string,
owner std.Address,
description string,
begin time.Time,
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

end time.Time,
minPrice uint64,
mous1985 marked this conversation as resolved.
Show resolved Hide resolved
) *Auction {
auction := &Auction{
title: title,
description: description,
owner: owner,
bids: avl.NewTree(),
begin: begin,
end: end,
price: minPrice,
}
// Emit AuctionStart event
std.Emit("AuctionStart", "title", title, "time", begin.String())
return auction
}

// GetTitle returns the title of the auction.
func (a Auction) GetTitle() string {
return a.title
}

// GetDescription returns the description of the auction.
func (a Auction) GetDescription() string {
return a.description
}

// GetOwner returns the address of the auction owner.
func (a Auction) GetOwner() std.Address {
return a.owner
}

// GetBegin returns the start time of the auction.
func (a Auction) GetBegin() time.Time {
return a.begin
}

// GetEnd returns the end time of the auction.
func (a Auction) GetEnd() time.Time {
return a.end
}

// GetPrice returns the current highest bid amount.
func (a Auction) GetPrice() uint64 {
return a.price
}

// 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 {
Copy link
Member

Choose a reason for hiding this comment

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

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

bids = append(bids, value.(*Bid))
return false
})
return bids
}

// IsOwner checks if the given address is the owner of the auction.
func (a Auction) IsOwner(address std.Address) bool {
mous1985 marked this conversation as resolved.
Show resolved Hide resolved
return address == a.GetOwner()
}

// NewBid creates a new Bid instance.
func NewBid(bidder std.Address, amount uint64) *Bid {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

return &Bid{
bidder: bidder,
amount: amount,
}
}

// AddBid adds a new bid to the auction.
func (a *Auction) AddBid(bidder std.Address, amount uint64) {
if amount <= a.price {
panic("bid amount must be higher than current highest bid")
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about returning errors instead, since this is a package after all?

Copy link
Author

Choose a reason for hiding this comment

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

I removed all panic and replaced it with errors

}
key := strconv.Itoa(a.bids.Size())
bid := NewBid(bidder, amount)
a.bids.Set(key, bid)
a.price = amount
std.Emit("BidPlaced", "bidder", bidder.String(), "amount", strconv.FormatUint(amount, 10))
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about exporting these event names as constants and using them this way?

}

// GetBidder returns the address of the bidder.
func (b Bid) GetBidder() std.Address {
return b.bidder
}

// GetAmount returns the amount of the bid.
func (b Bid) GetAmount() uint64 {
return b.amount
}

// EndAuction ends the auction and emits the AuctionEnded event.
func (a *Auction) EndAuction() {
if time.Now().Before(a.end) {
panic("Auction cannot end before the end time")
}
if a.bids.Size() == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

How is this a bad situation?

panic("No bids placed")
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

}
highestBid := a.GetPrice()
winner := a.GetBids()[a.bids.Size()-1].GetBidder()
std.Emit("AuctionEnded", "winner", winner.String(), "amount", strconv.FormatUint(highestBid, 10))
}
110 changes: 110 additions & 0 deletions examples/gno.land/p/demo/auction/auction_test.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package auction

import (
"std"
"testing"
"time"

"gno.land/p/demo/testutils"
)

// TestBid_new verifies that a new Bid can be initialized correctly
func TestBid_new(t *testing.T) {
var (
amount = uint64(100)
bidder = testutils.TestAddress("bidder")
)

// Create a new Bid instance
B := NewBid(bidder, amount)
if B.GetAmount() != amount {
t.Fatalf("invalid amount")
}
if B.GetBidder() != bidder {
t.Fatalf("invalid bidder")
}
}

// TestAuction_new verifies that a new Auction can be initialized correctly
func TestAuction_new(t *testing.T) {
var (
title = "auction title"
owner = testutils.TestAddress("owner")
description = "description"
begin = time.Now()
end = begin.Add(time.Hour * 24)
minPrice = uint64(1)
)

// Create a new Auction instance
A := NewAuction(title, owner, description, begin, end, minPrice)
if A.GetTitle() != title {
t.Fatalf("invalid title")
}
if A.GetOwner() != owner {
t.Fatalf("invalid owner")
}
if A.GetDescription() != description {
t.Fatalf("invalid description")
}
if A.GetBegin() != begin {
t.Fatalf("invalid begin")
}
if A.GetEnd() != end {
t.Fatalf("invalid end")
}
if A.GetPrice() != minPrice {
t.Fatalf("invalid minPrice")
}

std.TestSetOrigCaller(owner)
if !A.IsOwner(owner) {
t.Fatalf("invalid owner check")
}
}

// TestAuction_AddBid verifies that bids can be added to an auction correctly
func TestAuction_AddBid(t *testing.T) {
var (
title = "auction title"
owner = testutils.TestAddress("owner")
bidder1 = testutils.TestAddress("bidder1")
bidder2 = testutils.TestAddress("bidder2")
description = "description"
begin = time.Now()
end = begin.Add(time.Hour * 24)
minPrice = uint64(100)
)

// Create a new Auction instance
A := NewAuction(title, owner, description, begin, end, minPrice)

// to do Simulate time to be after auction start
// std.TestSetTime(begin.Add(time.Second))
Copy link
Member

Choose a reason for hiding this comment

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

Leftover?


// Place first bid
std.TestSetOrigCaller(bidder1)
A.AddBid(bidder1, 200)
if A.GetPrice() != 200 {
t.Fatalf("First bid failed")
}

// Place second bid
std.TestSetOrigCaller(bidder2)
A.AddBid(bidder2, 300)
if A.GetPrice() != 300 {
t.Fatalf("Second bid failed")
}

// Verify bids
bids := A.GetBids()
if len(bids) != 2 {
t.Fatalf("Expected 2 bids, got %d", len(bids))
}
if bids[0].GetBidder() != bidder1 || bids[0].GetAmount() != 200 {
t.Fatalf("First bid incorrect")
}
if bids[1].GetBidder() != bidder2 || bids[1].GetAmount() != 300 {
t.Fatalf("Second bid incorrect")
}
}
6 changes: 6 additions & 0 deletions examples/gno.land/p/demo/auction/gno.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module gno.land/p/demo/auction

require (
gno.land/p/demo/avl v0.0.0-latest
gno.land/p/demo/testutils v0.0.0-latest
)
134 changes: 134 additions & 0 deletions examples/gno.land/r/demo/auction/auction.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package auction

import (
"bytes"
"std"
"strconv"
"time"

auctionp "gno.land/p/demo/auction"
"gno.land/p/demo/avl"
"gno.land/p/demo/ufmt"
)

var (
auctionTree *avl.Tree
currentTime time.Time
)

// Initialize the realm with auction tree
func init() {
auctionTree = avl.NewTree()
}

// Mockable function to get current time
func now() time.Time {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, please drop currentTime

if !currentTime.IsZero() {
return currentTime
}
return time.Now()
}

// NewAuction creates a new auction in the realm
func NewAuction(
title string,
description string,
begin int64,
end int64,
price uint64,
) *auctionp.Auction {
txSender := std.GetOrigCaller()
Copy link
Member

Choose a reason for hiding this comment

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

I think you should use std.PrevRealm().Addr()


if begin < now().Unix() {
panic("begin has to be in the future")
}
if end <= now().Unix() {
panic("end has to be in the future")
}
if end <= begin {
panic("end has to be after begin")
}
if price <= 0 {
Copy link
Member

Choose a reason for hiding this comment

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

When is this possible, given that price is a uint64?

panic("price has to be positive")
}

auc := auctionp.NewAuction(
title,
txSender,
description,
time.Unix(begin, 0),
time.Unix(end, 0),
price,
)
id := strconv.Itoa(auctionTree.Size())
Copy link
Member

Choose a reason for hiding this comment

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

You can use the seqid package for sequential IDs, perfect for this AVL use case

auctionTree.Set(id, auc)
return auc
}

// AddBid places a bid in the auction
func AddBid(auctionID string, price uint64) string {
bidder := std.GetOrigCaller()
Copy link
Member

Choose a reason for hiding this comment

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

Same as with the previous comment

auc, ok := auctionTree.Get(auctionID)
if !ok {
panic("auction does not exist")
}
auction := auc.(*auctionp.Auction)
Copy link
Member

Choose a reason for hiding this comment

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

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

if auction.GetBegin().After(now()) {
panic("auction is not open yet")
}
if auction.GetEnd().Before(now()) {
panic("auction is closed")
}
if auction.IsOwner(bidder) {
panic("owner cannot bid")
}
if auction.GetPrice() >= price {
panic("price has to be higher than minimum price")
}

auction.AddBid(bidder, price)
return "bid placed"
}

// Render renders the state of the realm
func Render(path string) string {
if path == "" {
return renderHomepage()
}
return "unknown page"
}

// renderHomepage renders the homepage of the realm
func renderHomepage() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

var b bytes.Buffer
b.WriteString("<h1><center>Auctions</center></h1>\n\n")
Copy link
Member

Choose a reason for hiding this comment

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

Why not a simple #?

if auctionTree.Size() == 0 {
b.WriteString("## No auctions available\n")
return b.String()
}
auctionTree.Iterate("", "", func(key string, value interface{}) bool {
auc := value.(*auctionp.Auction)
b.WriteString("## " + auc.GetTitle() + "\n")
b.WriteString("### Owner: " + auc.GetOwner().String() + "\n")
b.WriteString("### Description: " + auc.GetDescription() + "\n\n")
b.WriteString("This auction starts on: " + auc.GetBegin().String() + " and ends on: " + auc.GetEnd().String() + "\n\n")
b.WriteString(ufmt.Sprintf("### Bids %d \n", auc.GetPrice()))
b.WriteString("## Bids\n")
bids := auc.GetBids()
for _, bid := range bids {
b.WriteString("Bidder: " + bid.GetBidder().String() + ", Amount: " + strconv.FormatUint(bid.GetAmount(), 10) + "\n")
}
return false
})
return b.String()
}

// Helper function to set the mock current time
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't need these

func setcurrentTime(t time.Time) {
currentTime = t
}

// Helper function to reset the mock current time
func resetcurrentTime() {
currentTime = time.Time{}
}
Loading
Loading