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(marketplace): Buy ORM #899

Merged
merged 64 commits into from
Mar 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
85c713e
feat(wip): marketplace Sell ORM
technicallyty Mar 10, 2022
a6f0bcc
Merge branch 'master' into ty/856-marketplace_ORM
technicallyty Mar 10, 2022
2644c5d
feat: sell
technicallyty Mar 10, 2022
531376d
refactor: move general utils
technicallyty Mar 10, 2022
356e511
feat: create market if not found
technicallyty Mar 10, 2022
8f768ad
chore: comments
technicallyty Mar 10, 2022
e4a818e
chore: remove iface guard comment
technicallyty Mar 10, 2022
23804d1
wip
technicallyty Mar 11, 2022
cfcd8f1
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 11, 2022
97244db
wip
technicallyty Mar 12, 2022
501ed4d
feat: escrow credits
technicallyty Mar 12, 2022
cb104de
fix: revert mock times
technicallyty Mar 12, 2022
0ed55f4
fix: typo
technicallyty Mar 12, 2022
d8957d0
Merge branch 'ty/821-escrow' into ty/856-buy_orm
technicallyty Mar 12, 2022
e81fef4
feat: balance update method
technicallyty Mar 12, 2022
1f901eb
chore: add event emission to sell
technicallyty Mar 12, 2022
863aa05
chore: test setup
technicallyty Mar 12, 2022
ad01737
fix: assert precision of sell order qty
technicallyty Mar 12, 2022
c124478
feat: more tests
technicallyty Mar 12, 2022
bf710e3
wip: prune
technicallyty Mar 14, 2022
d9576a8
feat: pruning
technicallyty Mar 14, 2022
cf7f9d3
chore: fmt.Errorf -> sdkerrors.ErrInvalidRequest
technicallyty Mar 15, 2022
06f65b7
Merge branch 'master' into ty/821-escrow
technicallyty Mar 15, 2022
70d0d40
Merge branch 'master' into ty/821-escrow
technicallyty Mar 15, 2022
4a52730
Merge branch 'ty/821-escrow' into ty/856-buy_orm
technicallyty Mar 15, 2022
a0291b9
chore: cleanup
technicallyty Mar 15, 2022
2ae117c
chore: add invalid test
technicallyty Mar 15, 2022
95a48c4
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 16, 2022
a809dc2
chore: add events
technicallyty Mar 16, 2022
3b8bfde
chore: go fmt
technicallyty Mar 16, 2022
0dd8475
chore: godoc
technicallyty Mar 16, 2022
0290e19
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 16, 2022
497e19a
chore: revert, comments, cleanup
technicallyty Mar 16, 2022
6c52726
chore: address review comments
technicallyty Mar 17, 2022
bb2f9ef
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 17, 2022
cbce516
chore: add test
technicallyty Mar 17, 2022
d93ae80
Merge branch 'ty/856-buy_orm' of https://github.com/regen-network/reg…
technicallyty Mar 17, 2022
5012b5e
chore: tidy imports
technicallyty Mar 17, 2022
89f21d8
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 17, 2022
97c7676
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 17, 2022
668a495
Merge branch 'ty/856-buy_orm' of https://github.com/regen-network/reg…
technicallyty Mar 17, 2022
81d2c29
Update x/ecocredit/server/marketplace/buy_test.go
technicallyty Mar 17, 2022
f93417a
Update x/ecocredit/server/marketplace/buy_test.go
technicallyty Mar 17, 2022
813b04d
Update x/ecocredit/server/marketplace/buy_test.go
technicallyty Mar 17, 2022
73ae70b
chore: review comments
technicallyty Mar 17, 2022
d714593
Merge branch 'ty/856-buy_orm' of https://github.com/regen-network/reg…
technicallyty Mar 17, 2022
7aee047
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 17, 2022
3993ab9
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 29, 2022
cfcec78
chore: ErrInvalidRequest -> ErrIO
technicallyty Mar 29, 2022
6241556
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 29, 2022
3164bdf
Merge branch 'ty/856-buy_orm' of https://github.com/regen-network/reg…
technicallyty Mar 29, 2022
52580a3
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 29, 2022
63ff796
chore: refactor purchase logic
technicallyty Mar 29, 2022
933449a
chore: fix conflicts
technicallyty Mar 29, 2022
48c2a72
fix: use correct error
technicallyty Mar 29, 2022
eb715aa
fix: charge correct amount of coins
technicallyty Mar 29, 2022
4e7d6ce
fix: check the coin send amount in test
technicallyty Mar 29, 2022
8f227d7
chore: address review
technicallyty Mar 30, 2022
3569f0f
Update x/ecocredit/server/marketplace/buy.go
technicallyty Mar 30, 2022
94520db
chore: update tests
technicallyty Mar 30, 2022
ec9804e
chore: cleanup error message
technicallyty Mar 30, 2022
f198a7c
chore: update error in test
technicallyty Mar 30, 2022
1c317da
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 30, 2022
591a4a4
Merge branch 'master' into ty/856-buy_orm
technicallyty Mar 30, 2022
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
Prev Previous commit
Next Next commit
chore: cleanup
  • Loading branch information
technicallyty committed Mar 15, 2022
commit a0291b97a063e9fbab1cd135cabc18865079cb00
60 changes: 41 additions & 19 deletions x/ecocredit/server/marketplace/buy.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
ecocreditv1 "github.com/regen-network/regen-ledger/api/regen/ecocredit/v1"
"github.com/regen-network/regen-ledger/types/math"
v1 "github.com/regen-network/regen-ledger/x/ecocredit/marketplace"
"github.com/regen-network/regen-ledger/x/ecocredit/server"
)

func (k Keeper) Buy(ctx context.Context, req *v1.MsgBuy) (*v1.MsgBuyResponse, error) {
aaronc marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -33,29 +34,37 @@ func (k Keeper) Buy(ctx context.Context, req *v1.MsgBuy) (*v1.MsgBuyResponse, er

switch selection := order.Selection.Sum.(type) {
case *v1.MsgBuy_Order_Selection_SellOrderId:
aaronc marked this conversation as resolved.
Show resolved Hide resolved
orderAmt, err := math.NewDecFromString(order.Quantity)
sellOrder, err := k.stateStore.SellOrderTable().Get(ctx, selection.SellOrderId)
if err != nil {
return nil, fmt.Errorf("sell order %d: %w", selection.SellOrderId, err)
}
batch, err := k.coreStore.BatchInfoTable().Get(ctx, sellOrder.BatchId)
if err != nil {
return nil, err
}
// get the sell order
sellOrder, err := k.stateStore.SellOrderStore().Get(ctx, selection.SellOrderId)
ct, err := server.GetCreditTypeFromBatchDenom(ctx, k.coreStore, k.params, batch.BatchDenom)
if err != nil {
return nil, fmt.Errorf("sell order %d: %w", selection.SellOrderId, err)
return nil, err
}
orderAmt, err := math.NewPositiveFixedDecFromString(order.Quantity, ct.Precision)
if err != nil {
return nil, err
}

if sellOrder.DisableAutoRetire != order.DisableAutoRetire {
return nil, sdkerrors.ErrInvalidRequest.Wrapf("auto-retire mismatch: sell order set to %t, buy "+
"order set to %t", sellOrder.DisableAutoRetire, order.DisableAutoRetire)
}
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
// get the market
market, err := k.stateStore.MarketStore().Get(ctx, sellOrder.MarketId)

market, err := k.stateStore.MarketTable().Get(ctx, sellOrder.MarketId)
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("market id %d: %w", sellOrder.MarketId, err)
}
// check that the order denoms match
if order.BidPrice.Denom != market.BankDenom {
return nil, sdkerrors.ErrInvalidRequest.Wrapf("bid price denom does not match ask price denom: "+
" %s, expected %s", order.BidPrice.Denom, market.BankDenom)
}

// check that bid price is at least equal to ask price
askAmount, ok := sdk.NewIntFromString(sellOrder.AskPrice)
if !ok {
Expand All @@ -65,6 +74,7 @@ func (k Keeper) Buy(ctx context.Context, req *v1.MsgBuy) (*v1.MsgBuyResponse, er
return nil, sdkerrors.ErrInsufficientFunds.Wrapf("bid price too low: got %s, needed at least %s",
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
order.BidPrice.Amount.String(), sellOrder.AskPrice)
}

if err = k.updateBalances(ctx, sellOrder, buyerAcc, orderAmt, *order.BidPrice); err != nil {
return nil, fmt.Errorf("error updating balances: %w", err)
}
aaronc marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -75,29 +85,37 @@ func (k Keeper) Buy(ctx context.Context, req *v1.MsgBuy) (*v1.MsgBuyResponse, er
return &v1.MsgBuyResponse{}, nil
}

// updateBalances moves credits according to the orders. it will:
// - update a sell order, removing it if quantity becomes 0 as a result of this purchase.
// - remove the purchaseQty from the seller's escrowed balance.
// - add credits to the buyer's tradable/retired address (based on the DisableAutoRetire field).
// - update the supply accordingly.
// - send the coins specified in the bid to the seller.
func (k Keeper) updateBalances(ctx context.Context, sellOrder *api.SellOrder, buyerAcc sdk.AccAddress, purchaseQty math.Dec, bidCoin sdk.Coin) error {
// update the sell order
sellOrderQty, err := math.NewDecFromString(sellOrder.Quantity)
if err != nil {
return err
}
// since we only support direct buy orders at this time, we fail when the requested purchase amount is more than
// the available credits for sale, rather than partial filling the buy order.
sellOrderQty, err = math.SafeSubBalance(sellOrderQty, purchaseQty)
if err != nil {
return err
}
if sellOrderQty.IsZero() { // remove the sell order
if err := k.stateStore.SellOrderStore().Delete(ctx, sellOrder); err != nil {
if sellOrderQty.IsZero() { // remove the sell order if no credits are left
if err := k.stateStore.SellOrderTable().Delete(ctx, sellOrder); err != nil {
return err
}
} else { // update the sell order
} else {
sellOrder.Quantity = sellOrderQty.String()
if err = k.stateStore.SellOrderStore().Update(ctx, sellOrder); err != nil {
if err = k.stateStore.SellOrderTable().Update(ctx, sellOrder); err != nil {
return err
}
}

// update the sellers balance
sellerBal, err := k.coreStore.BatchBalanceStore().Get(ctx, sellOrder.Seller, sellOrder.BatchId)
// remove the credits from the seller's escrowed balance
sellerBal, err := k.coreStore.BatchBalanceTable().Get(ctx, sellOrder.Seller, sellOrder.BatchId)
if err != nil {
return err
}
Expand All @@ -110,16 +128,16 @@ func (k Keeper) updateBalances(ctx context.Context, sellOrder *api.SellOrder, bu
return err
}
sellerBal.Escrowed = escrowBal.String()
if err = k.coreStore.BatchBalanceStore().Update(ctx, sellerBal); err != nil {
if err = k.coreStore.BatchBalanceTable().Update(ctx, sellerBal); err != nil {
return err
}

// update the buyers balance and supply
supply, err := k.coreStore.BatchSupplyStore().Get(ctx, sellOrder.BatchId)
supply, err := k.coreStore.BatchSupplyTable().Get(ctx, sellOrder.BatchId)
if err != nil {
return err
}
buyerBal, err := k.coreStore.BatchBalanceStore().Get(ctx, buyerAcc, sellOrder.BatchId)
buyerBal, err := k.coreStore.BatchBalanceTable().Get(ctx, buyerAcc, sellOrder.BatchId)
if err != nil {
if ormerrors.IsNotFound(err) {
buyerBal = &ecocreditv1.BatchBalance{
Expand All @@ -133,7 +151,7 @@ func (k Keeper) updateBalances(ctx context.Context, sellOrder *api.SellOrder, bu
return err
}
}
if sellOrder.DisableAutoRetire {
if sellOrder.DisableAutoRetire { // if auto retire is disabled, we move the credits into the buyer's/supply's tradable balance.
tradableBalance, err := math.NewDecFromString(buyerBal.Tradable)
if err != nil {
return err
Expand Down Expand Up @@ -174,17 +192,21 @@ func (k Keeper) updateBalances(ctx context.Context, sellOrder *api.SellOrder, bu
}
supply.RetiredAmount = supplyRetired.String()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

lot of repeated "decode + add". Would be good to create a helper function for that

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, why did we decide to store numbers as string rather than bytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, just how its been done so far. perhaps we need an issue/discussion on it

Copy link
Member

Choose a reason for hiding this comment

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

BTW, why did we decide to store numbers as string rather than bytes?

I thought we talked about this several times and everyone understood why. Since it's not clear can someone please make sure this is well documented so it doesn't keep coming up?

  1. we have no agreed upon standard for encoding big decimals or bid integers as bytes within the SDK, let alone in client applications
  2. decimal and integer types often need to be signed in transactions and need to be human readable for Ledger signing
  3. trying to find a solution for the above has always been seen as a premature optimization that will make debugging harder

at some point, a bytes standard may be useful, but for now I would consider it very low priority

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, let's continue in an issue - it's the same discussion as we have for addresses. Just for note that ledger eventually will use different serialization anyway.

// move subtract the purchased credits from the escrowed supply
// we can update the escrowed supply outside the condition since its the same either way
supplyEscrowed, err := math.NewDecFromString(supply.EscrowedAmount)
if err != nil {
return err
}
supplyEscrowed, err = math.SafeSubBalance(supplyEscrowed, purchaseQty)
if err != nil {
return err
}
supply.EscrowedAmount = supplyEscrowed.String()
if err = k.coreStore.BatchSupplyStore().Update(ctx, supply); err != nil {
if err = k.coreStore.BatchSupplyTable().Update(ctx, supply); err != nil {
return err
}
if err = k.coreStore.BatchBalanceStore().Save(ctx, buyerBal); err != nil {
if err = k.coreStore.BatchBalanceTable().Save(ctx, buyerBal); err != nil {
return err
}

Expand Down
60 changes: 56 additions & 4 deletions x/ecocredit/server/marketplace/buy_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package marketplace

import (
"github.com/cosmos/cosmos-sdk/orm/types/ormerrors"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -55,12 +56,12 @@ func TestBuy_ValidTradable(t *testing.T) {
assert.NilError(t, err)

// sell order should now have quantity 10 - 3 -> 7
sellOrder, err := s.marketStore.SellOrderStore().Get(s.ctx, 1)
sellOrder, err := s.marketStore.SellOrderTable().Get(s.ctx, 1)
assert.NilError(t, err)
assert.Equal(t, "7", sellOrder.Quantity)

// buyer didn't have credits before, so they should now have 3 credits
buyerBal, err := s.coreStore.BatchBalanceStore().Get(s.ctx, buyerAddr, 1)
buyerBal, err := s.coreStore.BatchBalanceTable().Get(s.ctx, buyerAddr, 1)
assert.NilError(t, err)
tradableBalance, err := math.NewDecFromString(buyerBal.Tradable)
assert.NilError(t, err)
Expand Down Expand Up @@ -109,14 +110,65 @@ func TestBuy_ValidRetired(t *testing.T) {
assert.NilError(t, err)

// sell order should now have quantity 10 - 3 -> 7
sellOrder, err := s.marketStore.SellOrderStore().Get(s.ctx, 1)
sellOrder, err := s.marketStore.SellOrderTable().Get(s.ctx, 1)
assert.NilError(t, err)
assert.Equal(t, "7", sellOrder.Quantity)

// buyer didn't have credits before, so they should now have 3 credits
buyerBal, err := s.coreStore.BatchBalanceStore().Get(s.ctx, buyerAddr, 1)
buyerBal, err := s.coreStore.BatchBalanceTable().Get(s.ctx, buyerAddr, 1)
assert.NilError(t, err)
retiredBalance, err := math.NewDecFromString(buyerBal.Retired)
assert.NilError(t, err)
assert.Check(t, retiredBalance.Equal(math.NewDecFromInt64(3)))
}

func TestBuy_OrderFilled(t *testing.T) {
t.Parallel()
s := setupBase(t)
_, _, buyerAddr := testdata.KeyTestPubAddr()
batchDenom := "C01-20200101-20200201-001"
start, end := timestamppb.Now(), timestamppb.Now()
ask := sdk.NewInt64Coin("ufoo", 10)
creditType := ecocredit.CreditType{
Name: "carbon",
Abbreviation: "C",
Unit: "tonnes",
Precision: 6,
}
marketTestSetup(t, s, batchDenom, ask.Denom, ask.Denom[1:], "C01", start, end, creditType)
// make a sell order
any := gomock.Any()
s.paramsKeeper.EXPECT().GetParamSet(any, any).Do(func(any interface{}, p *ecocredit.Params) {
p.CreditTypes = []*ecocredit.CreditType{&creditType}
}).AnyTimes()
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
sellExp := time.Now()
res, err := s.k.Sell(s.ctx, &marketplace.MsgSell{
Owner: s.addr.String(),
Orders: []*marketplace.MsgSell_Order{
{BatchDenom: batchDenom, Quantity: "10", AskPrice: &ask, DisableAutoRetire: false, Expiration: &sellExp},
},
})
assert.NilError(t, err)
sellOrderId := res.SellOrderIds[0]

s.bankKeeper.EXPECT().HasBalance(any, any, any).Return(true).Times(1)
s.bankKeeper.EXPECT().SendCoins(any, any, any, any).Return(nil).Times(1)

_, err = s.k.Buy(s.ctx, &marketplace.MsgBuy{
Buyer: buyerAddr.String(),
Orders: []*marketplace.MsgBuy_Order{
{Selection: &marketplace.MsgBuy_Order_Selection{Sum: &marketplace.MsgBuy_Order_Selection_SellOrderId{SellOrderId: sellOrderId}},
Quantity: "10", BidPrice: &ask, DisableAutoRetire: false, Expiration: &sellExp},
},
})
assert.NilError(t, err)

// order was filled, so sell order should no longer exist
_, err = s.marketStore.SellOrderTable().Get(s.ctx, sellOrderId)
assert.ErrorContains(t, err, ormerrors.NotFound.Error())
buyerBal, err := s.coreStore.BatchBalanceTable().Get(s.ctx, buyerAddr, 1)
assert.NilError(t, err)
retiredBal, err := math.NewDecFromString(buyerBal.Retired)
assert.NilError(t, err)
assert.Check(t, retiredBal.Equal(math.NewDecFromInt64(10)))
}
4 changes: 1 addition & 3 deletions x/ecocredit/server/marketplace/sell_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package marketplace

import (
"github.com/cosmos/cosmos-sdk/orm/types/ormerrors"
"github.com/regen-network/regen-ledger/types/math"
"github.com/regen-network/regen-ledger/x/ecocredit/server"

"testing"
"time"

Expand Down