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(x/ecocredit/marketplace): implement buyer and seller fees #2154

Merged
merged 31 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fa1208b
feat(x/ecocredit/marketplace): add marketplace fee API
aaronc Jan 30, 2024
3624c5d
docs
aaronc Jan 30, 2024
8e09e86
cleanup
aaronc Jan 30, 2024
d0bfd3e
proto-gen
aaronc Jan 30, 2024
00563af
Merge branch 'main' of github.com:regen-network/regen-ledger into aar…
aaronc Jan 31, 2024
c589b42
Merge branch 'main' of github.com:regen-network/regen-ledger into aar…
aaronc Jan 31, 2024
6459263
feat(x/ecocredit/marketplace): implement buyer and seller fees
aaronc Jan 31, 2024
d22fe85
validate basic
aaronc Jan 31, 2024
f82c581
add go-mod-tidy-all
aaronc Jan 31, 2024
0835631
WIP
aaronc Jan 31, 2024
c5b3f41
make existing tests pass
aaronc Jan 31, 2024
32d7bf1
failing test
aaronc Jan 31, 2024
64b093c
tests
aaronc Jan 31, 2024
494163f
Merge branch 'main' of github.com:regen-network/regen-ledger into aar…
aaronc Feb 5, 2024
aa7084d
fix tests
aaronc Feb 5, 2024
9e27574
fee params tests
aaronc Feb 5, 2024
77cfe06
set fee params validation tests
aaronc Feb 5, 2024
41b76f5
add failing max_fee_amount tests
aaronc Feb 5, 2024
dae82fa
tests passing
aaronc Feb 5, 2024
3ee7ed5
tests
aaronc Feb 5, 2024
c2a362e
lint
aaronc Feb 5, 2024
478f3e4
refactor max fee amount to coin
aaronc Feb 12, 2024
5ade325
Merge branch 'main' of github.com:regen-network/regen-ledger into aar…
aaronc Feb 28, 2024
c4a1c81
proto gen
aaronc Feb 28, 2024
b1a7ad2
add seller and buyer fee data to EventBuyDirect
aaronc Feb 28, 2024
7af0973
make max fee amount required
aaronc Feb 28, 2024
3376e76
add marketplace fee pool account
aaronc Feb 29, 2024
bd95846
fix tests
aaronc Feb 29, 2024
caa606c
add MsgGovSendFromFeePool
aaronc Feb 29, 2024
7442fe2
fix coins
aaronc Feb 29, 2024
dbf4e14
proto lint
aaronc Feb 29, 2024
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
79 changes: 64 additions & 15 deletions x/ecocredit/marketplace/keeper/features/msg_buy_direct.feature
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Feature: Msg/BuyDirect
Background:
Given a credit type
And alice created a sell order with ask denom "regen"
And bob has a bank balance with denom "regen"
And bob has bank balance "100regen"

Scenario: bid denom matches sell denom
When bob attempts to buy credits with bid denom "regen"
Expand All @@ -68,35 +68,35 @@ Feature: Msg/BuyDirect

Scenario Outline: buyer bank balance is greater than or equal to total cost (single buy order)
Given alice created a sell order with quantity "10" and ask amount "10"
And bob has a bank balance with amount "<balance-amount>"
And bob has bank balance "<balance-amount>"
When bob attempts to buy credits with quantity "10" and bid amount "10"
Then expect no error

Examples:
| description | balance-amount |
| greater than | 200 |
| equal to | 100 |
| greater than | 200regen |
| equal to | 100regen |

Scenario Outline: buyer bank balance is greater than or equal to total cost (multiple buy orders)
Given alice created two sell orders each with quantity "10" and ask amount "10"
And bob has a bank balance with amount "<balance-amount>"
And bob has bank balance "<balance-amount>"
When bob attempts to buy credits in two orders each with quantity "10" and bid amount "10"
Then expect no error

Examples:
| description | balance-amount |
| greater than | 400 |
| equal to | 200 |
| greater than | 400regen |
| equal to | 200regen |

Scenario: buyer bank balance is less than total cost (single buy order)
Given alice created a sell order with quantity "10" and ask amount "10"
And bob has a bank balance with amount "50"
And bob has bank balance "50regen"
When bob attempts to buy credits with quantity "10" and bid amount "10"
Then expect the error "orders[0]: quantity: 10, ask price: 10regen, total price: 100regen, bank balance: 50regen: insufficient funds"

Scenario: buyer bank balance is less than total cost (multiple buy orders)
Given alice created two sell orders each with quantity "10" and ask amount "10"
And bob has a bank balance with amount "150"
And bob has bank balance "150regen"
When bob attempts to buy credits in two orders each with quantity "10" and bid amount "10"
Then expect the error "orders[1]: quantity: 10, ask price: 10regen, total price: 100regen, bank balance: 50regen: insufficient funds"

Expand All @@ -105,7 +105,7 @@ Feature: Msg/BuyDirect
Background:
Given a credit type
And alice created a sell order with ask amount "10"
And bob has a bank balance with amount "100"
And bob has bank balance "100regen"

Scenario Outline: bid price greater than or equal to ask price
When bob attempts to buy credits with quantity "10" and bid amount "<bid-amount>"
Expand All @@ -125,7 +125,7 @@ Feature: Msg/BuyDirect
Background:
Given a credit type
And alice created a sell order with quantity "10"
And bob has a bank balance with amount "150"
And bob has bank balance "150regen"

Scenario Outline: quantity less than or equal to sell order quantity
When bob attempts to buy credits with quantity "<quantity>"
Expand Down Expand Up @@ -220,7 +220,7 @@ Feature: Msg/BuyDirect

Scenario: buyer bank balance updated
Given alice created a sell order with quantity "10" and ask price "10regen"
And bob has the bank balance "100regen"
And bob has bank balance "100regen"
When bob attempts to buy credits with quantity "10" and bid price "10regen"
Then expect bob bank balance "0regen"

Expand Down Expand Up @@ -344,12 +344,15 @@ Feature: Msg/BuyDirect
Background:
Given a credit type
And alice's address "regen1nzh226hxrsvf4k69sa8v0nfuzx5vgwkczk8j68"
And alice has bank balance "100regen"
And bob's address "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6"
And bob has bank balance "100regen"

Scenario: EventTransfer is emitted
Given alice created a sell order with id "1"
When bob attempts to buy credits with quantity "10"
Then expect event transfer with properties
Then expect no error
And expect event transfer with properties
"""
{
"sender": "regen1nzh226hxrsvf4k69sa8v0nfuzx5vgwkczk8j68",
Expand All @@ -363,7 +366,8 @@ Feature: Msg/BuyDirect
Scenario: EventRetire is emitted
Given alice created a sell order with id "1"
When bob attempts to buy credits with sell order id "1" and retirement reason "offsetting electricity consumption"
Then expect event retire with properties
Then expect no error
And expect event retire with properties
"""
{
"owner": "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6",
Expand All @@ -376,9 +380,54 @@ Feature: Msg/BuyDirect
Scenario: EventBuyDirect is emitted
Given alice created a sell order with id "1"
When bob attempts to buy credits with sell order id "1"
Then expect event buy direct with properties
Then expect no error
And expect event buy direct with properties
"""
{
"sell_order_id": 1
}
"""

Rule: buyer fees are deducted from buyer, seller fees are deducted from seller, and both go to fee pool
Background:
Given a credit type

Scenario: fees go to fee pool
Given alice created a sell order with quantity "10" and ask price "10foo"
* bob has bank balance "110foo"
* alice has bank balance "0foo"
* buyer fees are 0.1 and seller fees are 0.05
When bob attempts to buy credits with quantity "10" and bid price "10foo"
Then expect no error
* expect alice bank balance "95foo"
* expect bob bank balance "0foo"
* expect fee pool balance "15foo"

Scenario: fees get burned
Copy link
Contributor

Choose a reason for hiding this comment

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

Is an event emitted when fees are burned? I see above there are scenarios to test that specific events are emitted. Maybe that isn't needed here since it would not be an event type provided by marketplace submodule.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can emit the burn event here. Don't have a strong preference.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like a burn event should be emitted but it just isn't clear to me if that is already happening. Does the bank keeper do that for us? If so, then maybe we don't need to include in tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

The bank keeper does include burn events, but since MsgBuyDirect now also includes the fees paid and we can assume that if the denom is uregen that it was a burn, is it okay if we just leave it at that and not add another event?

Copy link

Choose a reason for hiding this comment

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

What are the pros and cons of adding a specific event here? seems like for users, it is useful to have a specific event.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if there's really a pro to adding it. The con is that it just adds duplicitous events which takes up extra storage. The bank keeper and marketplace module already emit events containing the relevant data, so I think we should leave it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I might have created some confusion here. I just wanted to make sure that a burn event does exist. I agree we do not need a duplicate event. This would all be very clear if the test scenario included checking that the burn event is emitted but no worries if this is non-trivial to add.

Given alice created a sell order with quantity "10" and ask price "20uregen"
* bob has bank balance "240uregen"
* alice has bank balance "0regen"
* buyer fees are 0.2 and seller fees are 0.1
When bob attempts to buy credits with quantity "10" and bid price "20uregen"
Then expect no error
* expect alice bank balance "180uregen"
* expect bob bank balance "0uregen"
* expect fee pool balance "0uregen"

Rule: max_fee_amount is enforced
Scenario Outline: max_fee_amount is enforced
Given a credit type
* alice created a sell order with quantity "10" and ask price "10foo"
* buyer fees are 0.2 and seller fees are 0.1
* bob has bank balance "200foo"
* bob sets a max fee of <max_fee_amount>
When bob attempts to buy credits with quantity "10" and bid price "20foo"
Then expect error contains "<error>"

Examples:
| max_fee_amount | error |
| 10 | max fee |
| 20 | |
| 30 | |


20 changes: 11 additions & 9 deletions x/ecocredit/marketplace/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ var (
)

type Keeper struct {
stateStore marketapi.StateStore
baseStore baseapi.StateStore
bankKeeper ecocredit.BankKeeper
authority sdk.AccAddress
stateStore marketapi.StateStore
baseStore baseapi.StateStore
bankKeeper ecocredit.BankKeeper
authority sdk.AccAddress
feePoolName string
}

func NewKeeper(ss marketapi.StateStore, cs baseapi.StateStore, bk ecocredit.BankKeeper,
authority sdk.AccAddress) Keeper {
authority sdk.AccAddress, feePoolName string) Keeper {
return Keeper{
baseStore: cs,
stateStore: ss,
bankKeeper: bk,
authority: authority,
baseStore: cs,
stateStore: ss,
bankKeeper: bk,
authority: authority,
feePoolName: feePoolName,
}
}
2 changes: 1 addition & 1 deletion x/ecocredit/marketplace/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func setupBase(t gocuke.TestingT, numAddresses int) *baseSuite {

authority, err := sdk.AccAddressFromBech32("regen1nzh226hxrsvf4k69sa8v0nfuzx5vgwkczk8j68")
assert.NilError(s.t, err)
s.k = NewKeeper(s.marketStore, s.baseStore, s.bankKeeper, authority)
s.k = NewKeeper(s.marketStore, s.baseStore, s.bankKeeper, authority, ecocredit.ModuleName)

// set test accounts
for i := 0; i < numAddresses; i++ {
Expand Down
37 changes: 32 additions & 5 deletions x/ecocredit/marketplace/keeper/msg_buy_direct.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,36 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
)
}

// check address has the total cost (price per * order quantity)
buyerBalance := k.bankKeeper.GetBalance(sdkCtx, buyerAcc, order.BidPrice.Denom)
cost, err := getTotalCost(sellOrderAskAmount, buyQuantity)
// calc sub-total (price per * order quantity)
subtotal, err := getSubTotalCost(sellOrderAskAmount, buyQuantity)
if err != nil {
return nil, err
}

// add buyer fees
feeParams, err := k.stateStore.FeeParamsTable().Get(ctx)
if err != nil {
return nil, err
}
totalCost := sdk.Coin{Amount: cost, Denom: market.BankDenom}
total, buyerFee, err := getTotalCostAndBuyerFee(subtotal, feeParams)
totalCost := sdk.Coin{Amount: total.SdkIntTrim(), Denom: market.BankDenom}

// check max fee
if maxFee := order.MaxFeeAmount; maxFee != "" {
maxFeeInt, ok := sdk.NewIntFromString(maxFee)
if !ok {
return nil, sdkerrors.ErrInvalidType.Wrapf("could not convert %s to %T", maxFee, sdkmath.Int{})
}
if maxFeeInt.LT(buyerFee.SdkIntTrim()) {
return nil, sdkerrors.ErrInvalidRequest.Wrapf(
"%s: max fee: %s, required fee: %s",
orderIndex, maxFee, buyerFee,
)
}
}

// check address has the total cost
buyerBalance := k.bankKeeper.GetBalance(sdkCtx, buyerAcc, order.BidPrice.Denom)
if buyerBalance.IsLT(totalCost) {
return nil, sdkerrors.ErrInsufficientFunds.Wrapf(
"%s: quantity: %s, ask price: %s%s, total price: %v, bank balance: %v",
Expand All @@ -113,11 +136,15 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
sellOrder: sellOrder,
buyerAcc: buyerAcc,
buyQuantity: buyQuantity,
totalCost: totalCost,
totalCost: total,
subTotalCost: subtotal,
buyerFee: buyerFee,
autoRetire: !order.DisableAutoRetire,
batchDenom: batch.Denom,
bankDenom: market.BankDenom,
jurisdiction: order.RetirementJurisdiction,
reason: order.RetirementReason,
feeParams: feeParams,
}); err != nil {
return nil, err
}
Expand Down
Loading
Loading