Skip to content

Commit

Permalink
[1658]: Increase the maximmum external id length to allow for 32 byte…
Browse files Browse the repository at this point in the history
… bech32 addresses.
  • Loading branch information
SpicyLemon committed Oct 13, 2023
1 parent 994815e commit d03a9b2
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 16 deletions.
6 changes: 3 additions & 3 deletions docs/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1803,7 +1803,7 @@ AskOrder represents someone's desire to sell something at a minimum price.
| `price` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | | price is the minimum amount that the seller is willing to accept for the assets. The seller's settlement proportional fee (and possibly the settlement flat fee) is taken out of the amount the seller receives, so it's possible that the seller will still receive less than this price. |
| `seller_settlement_flat_fee` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | | seller_settlement_flat_fee is the flat fee for sellers that will be charged during settlement. If this denom is the same denom as the price, it will come out of the actual price received. If this denom is different, the amount must be in the seller's account and a hold is placed on it until the order is filled or cancelled. |
| `allow_partial` | [bool](#bool) | | allow_partial should be true if partial fulfillment of this order should be allowed, and should be false if the order must be either filled in full or not filled at all. |
| `external_id` | [string](#string) | | external_id is an optional string used to externally identify this order. Max length is 40 characters. If an order in this market with this external id already exists, this order will be rejected. |
| `external_id` | [string](#string) | | external_id is an optional string used to externally identify this order. Max length is 100 characters. If an order in this market with this external id already exists, this order will be rejected. |



Expand All @@ -1824,7 +1824,7 @@ BidOrder represents someone's desire to buy something at a specific price.
| `price` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | | price is the amount that the buyer will pay for the assets. A hold is placed on this until the order is filled or cancelled. |
| `buyer_settlement_fees` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | buyer_settlement_fees are the fees (both flat and proportional) that the buyer will pay (in addition to the price) when the order is settled. A hold is placed on this until the order is filled or cancelled. |
| `allow_partial` | [bool](#bool) | | allow_partial should be true if partial fulfillment of this order should be allowed, and should be false if the order must be either filled in full or not filled at all. |
| `external_id` | [string](#string) | | external_id is an optional string used to externally identify this order. Max length is 40 characters. If an order in this market with this external id already exists, this order will be rejected. |
| `external_id` | [string](#string) | | external_id is an optional string used to externally identify this order. Max length is 100 characters. If an order in this market with this external id already exists, this order will be rejected. |



Expand Down Expand Up @@ -2255,7 +2255,7 @@ MsgMarketSetOrderExternalIDRequest is a request message for the MarketSetOrderEx
| `admin` | [string](#string) | | admin is the account with "set_ids" permission requesting this settlement. |
| `market_id` | [uint32](#uint32) | | market_id is the numerical identifier of the market to update required attributes for. |
| `order_id` | [uint64](#uint64) | | order_id is the numerical identifier of the order to update. |
| `external_id` | [string](#string) | | external_id is the new external id to associate with the order. Max length is 40 characters. If the external id is already associated with another order in this market, this update will fail. |
| `external_id` | [string](#string) | | external_id is the new external id to associate with the order. Max length is 100 characters. If the external id is already associated with another order in this market, this update will fail. |



Expand Down
4 changes: 2 additions & 2 deletions proto/provenance/exchange/v1/orders.proto
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ message AskOrder {
// allow_partial should be true if partial fulfillment of this order should be allowed, and should be false if the
// order must be either filled in full or not filled at all.
bool allow_partial = 6;
// external_id is an optional string used to externally identify this order. Max length is 40 characters.
// external_id is an optional string used to externally identify this order. Max length is 100 characters.
// If an order in this market with this external id already exists, this order will be rejected.
string external_id = 7;
}
Expand All @@ -72,7 +72,7 @@ message BidOrder {
// allow_partial should be true if partial fulfillment of this order should be allowed, and should be false if the
// order must be either filled in full or not filled at all.
bool allow_partial = 6;
// external_id is an optional string used to externally identify this order. Max length is 40 characters.
// external_id is an optional string used to externally identify this order. Max length is 100 characters.
// If an order in this market with this external id already exists, this order will be rejected.
string external_id = 7;
}
2 changes: 1 addition & 1 deletion proto/provenance/exchange/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ message MsgMarketSetOrderExternalIDRequest {

// order_id is the numerical identifier of the order to update.
uint64 order_id = 3;
// external_id is the new external id to associate with the order. Max length is 40 characters.
// external_id is the new external id to associate with the order. Max length is 100 characters.
// If the external id is already associated with another order in this market, this update will fail.
string external_id = 4;
}
Expand Down
13 changes: 7 additions & 6 deletions x/exchange/keeper/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper_test

import (
"bytes"
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -3746,11 +3747,11 @@ func TestMakeIndexKeyMarketExternalIDToOrder(t *testing.T) {
expPanic: "cannot create market external id to order index with empty external id",
},
{
name: "41 char external id",
name: "external id too long",
marketID: 2,
externalID: "This id has forty-one chars. That's long!",
expPanic: "cannot create market external id to order index: invalid external id " +
"\"This id has forty-one chars. That's long!\": max length 40",
externalID: strings.Repeat("H", exchange.MaxExternalIDLength+1),
expPanic: fmt.Sprintf("cannot create market external id to order index: invalid external id %q: max length %d",
strings.Repeat("H", exchange.MaxExternalIDLength+1), exchange.MaxExternalIDLength),
},
{
name: "market 0, a zeroed uuid",
Expand All @@ -3775,9 +3776,9 @@ func TestMakeIndexKeyMarketExternalIDToOrder(t *testing.T) {
{
name: "max market id and lots of Zs",
marketID: 4_294_967_295,
externalID: strings.Repeat("Z", 40),
externalID: strings.Repeat("Z", exchange.MaxExternalIDLength),
expected: append([]byte{keeper.KeyTypeMarketExternalIDToOrderIndex, 255, 255, 255, 255},
strings.Repeat("Z", 40)...),
strings.Repeat("Z", exchange.MaxExternalIDLength)...),
},
}

Expand Down
7 changes: 6 additions & 1 deletion x/exchange/orders.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ const (
OrderTypeByteBid = byte(0x01)
)

const MaxExternalIDLength = 40
// MaxExternalIDLength is the maximum length that an external id can have.
// A 32 byte address as a bech32 string is 59 characters + the hrp.
// E.g. a 32 byte address with hrp "pb" will be 61 characters long.
// Technically, a bech32 HRP can be 1 to 83 characters. This 100 was chosen as a balance meant
// to allow most of those while still limiting the length of keys that use these external ids.
const MaxExternalIDLength = 100

// SubOrderI is an interface with getters for the fields in a sub-order (i.e. AskOrder or BidOrder).
type SubOrderI interface {
Expand Down
4 changes: 2 additions & 2 deletions x/exchange/orders.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/exchange/tx.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d03a9b2

Please sign in to comment.