New endpoint to support trading fees on limit orders for Openfund and Focus #640

diamondhands0 · 2024-06-14T22:53:21Z

Fees can be added to a market by the profile owner.
When fees are set, orders created for a market will automatically include fees.
All of the return values are optimized for frontend display, so that no math or logic needs to live outside of this endpoint.
Enables charging fees on Openfund and Focus.
Thoroughly tested by amm-service chain_utils.go (~100% coverage)

* Allow ParamUpdater to update PoS GlobalParams. * Add epoch number to get pos params call. * Change core branch in test Dockerfile. * Add test for updating global params. * Fix bugs in testing updating global params. * Checkout correct core branch. * Change core branch.

* Add validator registration endpoints. * Allow ExtraData to be logged.

* Add get validator by PublicKey route. * Address PR review feedback.

* Rename VotingPublicKeySignature to VotingAuthorization. * Checkout corresponding core branch. * Change core branch in test CI.

* Refactor merging GlobalParamsEntry defaults. * Change core branch in test dockerfile.

…#512) Co-authored-by: Lazy Nina <>

…#513) Co-authored-by: Lazy Nina <>

Co-authored-by: Lazy Nina <>

* [stable] Release 3.4.4 * add node version endpoint (#505) Co-authored-by: Lazy Nina <> * [stable] Release 3.4.5 * hotfix to exchange test * Add captcha verification (#509) * Updates to captcha verification * Updates to backend * Updates to captcha verify * Update captcha verification * Cleanup logs * Add routes to store reward amount in global state, track usage via data dog * Update verify captcha validation ordering, add back comp profile config bool * Update badger sync settings to optimize memory usage during hypersync (#506) * Update hypersync to use default badger settings, switch to performance settings once hypersync completes * Update test dockerfile to accept core branch name as parameter * Blank commit to trigger build * ln/fix-transaction-info-mempool (#510) Co-authored-by: Lazy Nina <> * ln/no-comp-when-0-create-profile-fee (#511) Co-authored-by: Lazy Nina <> * Empty commit to trigger build (#515) * Add extra data to basic transfer and diamond txn construction endpoints (#516) * trigger build (#517) Co-authored-by: Lazy Nina <> * trigger build * Add RWLock around AllCountryLevelSignUpBonuses (#518) Co-authored-by: Lazy Nina <> --------- Co-authored-by: Lazy Nina <> Co-authored-by: superzordon <88362450+superzordon@users.noreply.github.com>

Co-authored-by: Lazy Nina <>

…nstruction endpoint (#523) Co-authored-by: Lazy Nina <>

* Add stake, unstake, and unlock stake txn construction endpoints * Add stake, unstake, and unlock stake txn construction endpoints --------- Co-authored-by: Lazy Nina <>

Co-authored-by: Lazy Nina <>

…529)

* Add spending limits backend support for stake, unstake, unlock stake * Add txn construction and get endpoints for lockups * Add additional sanity checks to lockup endpoint. * Add txn construction and get endpoints for lockups * Add additional sanity checks to lockup endpoint. * Remove redundant profile entry response from LockedBalanceEntryResponse. * Add proper timestamp to simulateSubmitTransaction. * Apply suggestions from code review --------- Co-authored-by: Lazy Nina <> Co-authored-by: Jon Pollock <jon@deso.org>

…diamondhands/TRADING-FEES

diamondhands0 · 2024-06-28T17:25:25Z

routes/transaction.go

 		}
 	}

+	if len(transactionSpendingLimit.StakeLimitMap) > 0 {


Not sure why this is here. Will take a look and remove...

Look up above - this looks like it's duplicated. Confirm that it's the same and then just delete it.

I think we can delete all these changes to the TransactionSpendingLimitToResponse function.

Confirmed they were duplicate and deleted.

lazynina · 2024-06-28T17:55:22Z

routes/dao_coin_exchange.go

+	quantityToFill string,
+) (*uint256.Int, error) {
+	// If we don't return zero here, we error later because it thinks we overflowed
+	if quantityToFill == "0" || quantityToFill == "0.0" {


What if someone passes 0.00? Should we do some sort of strconv.ParseFloat?

Good one. Fixed.

lazynina · 2024-06-28T18:15:13Z

routes/dao_coin_exchange_with_fees.go

+	// assumed to own the profile being updated.
+	ProfilePublicKeyBase58Check string `safeForLogging:"true"`
+
+	// A map of pkid->feeBasisPoints that the user wants to set for their market.


Do you mean a map of public key to fee basis points? All API requests should assume we're receiving public keys as we don't expose PKIDs from the backend. This could lead to unexpected results. What you can do is have the backend convert these from public key to PKID to be safe.

Yep good point. I was a little sloppy on pkid vs pubkey so I'll go through and make sure all the API endpoints only expose pubkey, and that the conversion to pkid is correct in all places.

routes/dao_coin_exchange_with_fees.go

lazynina · 2024-06-28T18:19:02Z

routes/dao_coin_exchange_with_fees.go

+
+	// When this is nil then the UpdaterPublicKey is assumed to be the owner of
+	// the profile.
+	var profilePublicKeyBytess []byte


Suggested change

var profilePublicKeyBytess []byte

var profilePublicKeyBytes []byte

This is only useful for param updater, right?

lazynina · 2024-06-28T18:56:55Z

routes/dao_coin_exchange_with_fees.go

+		}
+
+		return res, nil
+	} else {


Maybe we can break this if and else into two separate functions? There's a LOT of logic above this and it would take me quite a while in the future to ever address any bugs in this endpoint.

Let's do it later.

lazynina · 2024-06-28T18:57:24Z

routes/dao_coin_exchange_with_fees.go

+	error,
+) {
+
+	return nil, fmt.Errorf("HandleLimitOrder: Not implemented")


What are we doing here?

Whoops; this was a leftover stub.

routes/server.go

lazynina · 2024-06-28T19:00:59Z

routes/transaction.go

-			return
-		}
-	}
+	txnn, totalInputt, spendAmountt, changeAmountt, feeNanoss, err := fes.CreateSendDesoTxn(


Appreciate the refactor here.

lazynina · 2024-06-28T19:02:59Z

routes/transaction.go

 		}
 	}

+	if len(transactionSpendingLimit.StakeLimitMap) > 0 {


I think we can delete all these changes to the TransactionSpendingLimitToResponse function.

…diamondhands/TRADING-FEES

…rotocol/backend into diamondhands/TRADING-FEES

…diamondhands/TRADING-FEES

lazynina · 2024-07-12T21:02:00Z

routes/dao_coin_exchange_with_fees.go

+		existingProfileEntry.IsHidden,  // Don't update hidden status
+		0,                              // Don't add additionalFees
+		existingProfileEntry.ExtraData, // The new ExtraData
+		requestData.MinFeeRateNanosPerKB, fes.backendServer.GetMempool(), []*lib.DeSoOutput{})


CreateDAOCoinLimitOrder DOES have TransactionFees. This breaks some functionality we created for node operators to be able to add a basic transfer output when somebody is constructing a txn on their node.

lazynina · 2024-07-12T21:03:08Z

routes/dao_coin_exchange_with_fees.go

+					"GetTradingFeesForMarket: Problem decoding public key %s: %v",
+					pkid, err)
+			}
+			pubkey := lib.PKIDToPublicKey(lib.NewPKID(pkidBytes))


This is incorrect - we want to use a utxo view to fetch the public key for the given PKID instead of just converting the PKID to a public key. If a public key had its identity swapped, then this would behave unexpectedly.

Whoops! Good catch I fixed it. Don't think there are any other places where this was an issue.

lazynina · 2024-07-12T21:04:12Z

routes/dao_coin_exchange_with_fees.go

+	_baseCurrencyPkid string,
+	_err error,
+) {
+	// The rule of thumb is we're seeling the base with an ask and buying the


Suggested change

// The rule of thumb is we're seeling the base with an ask and buying the

// The rule of thumb is we're selling the base with an ask and buying the

lazynina · 2024-07-12T21:06:15Z

routes/dao_coin_exchange_with_fees.go

+
+	// If the profile is the dusdc profile then just return 1.0
+	lowerUsername := strings.ToLower(string(existingProfileEntry.Username))
+	if strings.Contains(lowerUsername, "dusdc") {


Just checking the username is insufficient. someone could make a new username that includes dusdc and could lead to unexpected results. Can we add a validation to ensure we have approved stuff coming in for quoteCurrencyPublicKey?

Ah you're right! I just changed it to an exact match, which I believe should be robust to this. I think before I wasn't sure if we'd have the same setup on devnet/testnet/mainnet, but we do so it's fine.

lazynina · 2024-07-12T21:07:49Z

routes/dao_coin_exchange_with_fees.go

+				lib.PkToString(order.BuyingDAOCoinCreatorPKID[:], fes.Params),
+				lib.PkToString(order.SellingDAOCoinCreatorPKID[:], fes.Params),


I think this is ultimately fine, but we really should be looking up the public key for PKIDs instead of just converting them.

Yeah I left this one as-is because it's an isolated case where it's just checking for deso pkid.

lazynina · 2024-07-12T21:11:54Z

routes/dao_coin_exchange_with_fees.go

+		Price:                priceStrConsensus,
+		FillType:             req.FillType,
+		MinFeeRateNanosPerKB: req.MinFeeRateNanosPerKB,
+		TransactionFees:      req.TransactionFees,


ditto on this.

lazynina · 2024-07-12T21:12:43Z

routes/dao_coin_exchange_with_fees.go

+			//
+			// TODO: Add ExtraData to the transaction to make it easier to report it as an
+			// earning to the user who's receiving the fee.
+			txn, err := fes.SendCoins(


alright that's fine - it just becomes more expensive to assess fees.

lazynina · 2024-07-12T21:13:16Z

routes/dao_coin_exchange_with_fees.go

+			Price:                priceStrQuoteInverted,
+			FillType:             req.FillType,
+			MinFeeRateNanosPerKB: req.MinFeeRateNanosPerKB,
+			TransactionFees:      req.TransactionFees,


should merge in the node-level configured transaction fees.

I fixed this.

lazynina · 2024-07-12T21:15:27Z

routes/dao_coin_exchange_with_fees.go

+					DAOCoinToTransferNanos: *feeBaseUnits,
+				},
+				// Standard transaction fields
+				req.MinFeeRateNanosPerKB, fes.backendServer.GetMempool(), nil)


once again this breaks this node-level transaction fee stuff we worked on.

lazynina · 2024-07-12T21:17:01Z

routes/transaction.go

+	// committed tip height. Otherwise we'll be missing ~2 blocks in limbo.
+	coreChainTipHeight := fes.TXIndex.CoreChain.BlockTip().Height
+	for fes.TXIndex.TXIndexChain.BlockTip().Height < coreChainTipHeight {
+		if time.Since(startTime) > 30*time.Second {


Can we lower this value?

It's a maximum value, and it will always always return faster than this. That said, it can sometimes take a long time for txindex to catch up and I think it's good to leave it at this value.

lazynina · 2024-07-13T00:41:07Z

routes/dao_coin_exchange_with_fees.go


+	// Compute the additional transaction fees as specified by the request body and the node-level fees.
+	additionalOutputs, err := fes.getTransactionFee(
+		lib.TxnTypeAuthorizeDerivedKey, updaterPublicKeyBytes, requestData.TransactionFees)


This should be TxnTypeUpdateProfile

Whoops! Fixed.

lazynina · 2024-07-13T00:41:34Z

routes/dao_coin_exchange_with_fees.go

 	// If the profile is the dusdc profile then just return 1.0
 	lowerUsername := strings.ToLower(string(existingProfileEntry.Username))
-	if strings.Contains(lowerUsername, "dusdc") {
+	if lowerUsername == "dusdc_" {


I like this much better! we just need to remember to adjust it if we ever change the profile name.

lazynina · 2024-07-13T00:48:21Z

routes/dao_coin_exchange_with_fees.go

+		// from the transactor directly to the person receiving the fee.
+		transferTxns := []*lib.MsgDeSoTxn{}
+		for pkid, feeBaseUnits := range feeBaseUnitsByPubkey {
+			receiverPubkeyBytes, _, err := lib.Base58CheckDecode(pkid)


@diamondhands0 - do we need to go from PKID to public key here?

Yeah this one's just a loop variable but I'll do one more pass before merging don't worry.

…diamondhands/TRADING-FEES

mattfoley8 and others added 30 commits April 7, 2023 13:32

Test backend CI against core with relic dependency.

82b9729

Checkout specific branch just for testing purposes.

6834066

Update test Dockerfile.

6a65766

Merge branch 'main' into mf/add-bls-signature-utils

6fca7de

Fix failing build stages.

5428fa7

Merge branch 'main' into mf/add-bls-signature-utils

67ee851

Add new deps.

cc55761

Mf/add validator registration endpoints (#500)

898b8aa

* Add validator registration endpoints. * Allow ExtraData to be logged.

Add relic dependency to prod dockerfile.

cf62ed0

Merge mf/add-bls-signature-utils. Resolve conflicts.

23a9171

Mf/add get validator by public key endpoint (#501)

32e6c56

* Add get validator by PublicKey route. * Address PR review feedback.

Rename VotingPublicKeySignature to VotingAuthorization. (#502)

1f88c52

* Rename VotingPublicKeySignature to VotingAuthorization. * Checkout corresponding core branch. * Change core branch in test CI.

Refactor merging GlobalParamsEntry defaults. (#503)

a64baa6

* Refactor merging GlobalParamsEntry defaults. * Change core branch in test dockerfile.

Update test.Dockerfile core branch. (#504)

ea9c5d5

Initial backend updates to conform to new function signatures in core (…

c40453c

…#512) Co-authored-by: Lazy Nina <>

Update backend to use BackendMempool interface instead of DeSoMempool (…

f045518

…#513) Co-authored-by: Lazy Nina <>

Add fee estimator arg to all txn construction calls (#514)

f3932da

Co-authored-by: Lazy Nina <>

Remove fee estimator from txn construction calls (#520)

e392f7f

Co-authored-by: Lazy Nina <>

Fix compilation errors (#521)

c70fa5c

Co-authored-by: Lazy Nina <>

Fix validator test (#522)

753f14d

Co-authored-by: Lazy Nina <>

Add DelegatedStakeCommissionBasisPoints to RegisterAsValidator txn co…

43493e3

…nstruction endpoint (#523) Co-authored-by: Lazy Nina <>

Add stake, unstake, and unlock stake txn construction endpoints (#524)

c5f420c

* Add stake, unstake, and unlock stake txn construction endpoints * Add stake, unstake, and unlock stake txn construction endpoints --------- Co-authored-by: Lazy Nina <>

Add GET endpoints for stake and locked stake entries (#525)

6b5ddf3

Co-authored-by: Lazy Nina <>

Merge branch 'main' into feature/proof-of-stake

36ec3e6

Add spending limits backend support for stake, unstake, unlock stake (#…

91b9b8c

…529)

Update Block Header Timestamps to int64 (#535)

0cf96c1

Fix Backend To Run With Regtest PoS Node (#536)

c11ca20

diamondhands0 added 3 commits June 14, 2024 16:15

Incorporate core changes

3fadbc6

fix nit

abab54e

Fix deps and minor bug fixes

ab0b282

Base automatically changed from feature/proof-of-stake to main June 17, 2024 18:05

diamondhands0 added 3 commits June 17, 2024 16:02

Merge remote-tracking branch 'origin' into diamondhands/TRADING-FEES

38a0416

Merge branch 'main' of https://github.com/deso-protocol/backend into …

95baafa

…diamondhands/TRADING-FEES

fix test.Dockerfile

b288482

diamondhands0 commented Jun 28, 2024

View reviewed changes

lazynina requested changes Jun 28, 2024

View reviewed changes

lazynina and others added 13 commits June 28, 2024 15:12

Merge branch 'main' into diamondhands/TRADING-FEES

0adf104

Merge branch 'main' of https://github.com/deso-protocol/backend into …

a8837ff

…diamondhands/TRADING-FEES

Merge branch 'diamondhands/TRADING-FEES' of https://github.com/deso-p…

c5c2958

…rotocol/backend into diamondhands/TRADING-FEES

Trigger build

11cfc11

Fix usd price computation in market order and add price endpoint

c1a9e13

Fix bug around quantity currency

4c5d527

Fix txindex bug

708c898

Trigger rebuild

3867244

Merge branch 'main' of https://github.com/deso-protocol/backend into …

3a3f2cb

…diamondhands/TRADING-FEES

fix gettxn

a1d5382

Merge branch 'main' of https://github.com/deso-protocol/backend into …

962aea1

…diamondhands/TRADING-FEES

review comments

b130e89

Merge branch 'main' of https://github.com/deso-protocol/backend into …

a54fa14

…diamondhands/TRADING-FEES

lazynina requested changes Jul 12, 2024

View reviewed changes

Review fixes

d893a96

lazynina approved these changes Jul 13, 2024

View reviewed changes

lazynina reviewed Jul 13, 2024

View reviewed changes

diamondhands0 added 2 commits July 12, 2024 18:03

Patch last pkid issue

798111a

Merge branch 'main' of https://github.com/deso-protocol/backend into …

e390f9f

…diamondhands/TRADING-FEES

diamondhands0 merged commit 5130639 into main Jul 13, 2024

diamondhands0 deleted the diamondhands/TRADING-FEES branch July 13, 2024 02:43

	var profilePublicKeyBytess []byte
	var profilePublicKeyBytes []byte

	// The rule of thumb is we're seeling the base with an ask and buying the
	// The rule of thumb is we're selling the base with an ask and buying the

		lib.PkToString(order.BuyingDAOCoinCreatorPKID[:], fes.Params),
		lib.PkToString(order.SellingDAOCoinCreatorPKID[:], fes.Params),

New endpoint to support trading fees on limit orders for Openfund and Focus #640

New endpoint to support trading fees on limit orders for Openfund and Focus #640

Uh oh!

Conversation

diamondhands0 commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diamondhands0 commented Jun 14, 2024 •

edited

Loading