Jp/subsidized update profile #584
Conversation
* 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 <[email protected]>
Co-authored-by: Lazy Nina <>
Co-authored-by: Lazy Nina <>
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 <>
* 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 <[email protected]>
* Simple CreateAtomicTxnsWrapper scaffolding. * Update method on CreateAtomicTxnsWrapper. * Use lib.GetAugmentedUniversalViewWithAdditionalTransactions for UpdateProfile. * Use MaxTxnSizeBytesPoS for atomic transaction construction. * Use updated lib.GetAugmentedUniversalViewWithAdditionalTransactions parameters. * Gate preceding transactions middleware on POST requests. * Use anonymous struct to simplify preceding transaction middleware. * Update stateful transaction endpoints to use GetAugmentedUniversalViewWithAdditionalTransactions. * Simple CreateAtomicTxnsWrapper scaffolding. * Update method on CreateAtomicTxnsWrapper. * Use MaxTxnSizeBytesPoS for atomic transaction construction. * Fix MaxTxnSizeBytes bug --------- Co-authored-by: diamondhands <[email protected]> Co-authored-by: Lazy Nina <[email protected]>
…truction endpoint (#583)
| // is equivalent to the sum of the fees paid and the sum of the basic transfers (outputs). | ||
| func ComputeNecessarySubsidyForTransaction(txn *lib.MsgDeSoTxn) uint64 { | ||
| necessarySubsidy := txn.TxnFeeNanos | ||
| for _, txOutput := range txn.TxOutputs { |
There was a problem hiding this comment.
We must delete this otherwise the wallet can be drained instantaneously.
There was a problem hiding this comment.
I wasn't sure this would be accessible to the frontend. Isn't this a risk only if we accept transactions to subsidize from the frontend?
Currently transactions are constructed AND subsidized in the backend in the same endpoint, meaning ComputeNecessarySubsidyForTransaction should be safe. Happy to change it if we want though, could be bad to have this in general.
There was a problem hiding this comment.
I make a request to subsidized-update-profile and add TransactionFees: [{ PublicKeyBase58Check: 'some-public-key', AmountNanos: 10*1e9}]. Now in line 875 of your transaction.go file, this gets added as an additional output in updateProfileTxn. updateProfileTxn is passed into ComputeNecessarySubsidyForTransaction and I've just taken 10 DESO from the subsidy wallet.
There was a problem hiding this comment.
Ahh yeah you're correct... didn't realize the transaction request data enabled the user to specify the transaction fees. Assumed we would compute that on the backend based on network conditions.
There was a problem hiding this comment.
yeah they aren't network fees really - they allow app developers to easily request a amount of DESO when using their app to transact.
| requestData.IsHidden, | ||
| additionalFees, | ||
| extraData, | ||
| requestData.MinFeeRateNanosPerKB, |
There was a problem hiding this comment.
We should force this txn to be constructed with the global min fee rate, otherwise the subsidizing wallet could be drained very quickly.
| fmt.Sprintf("SubmitAtomicTransaction: "+ | ||
| "Problem deserializing atomic transaction from bytes: %v", err)) | ||
| return | ||
| } | ||
| if atomicTxn.TxnMeta.GetTxnType() != lib.TxnTypeAtomicTxnsWrapper { | ||
| _AddBadRequestError(ww, fmt.Sprintf("SubmitAtomicTransaction: "+ | ||
| "IncompleteAtomicTransaction must be an atomic transaction")) | ||
| return | ||
| } | ||
|
|
||
| // Create a map from the pre-signature inner transaction hash to DeSo signature. | ||
| innerTxnPreSignatureHashToSignature := make(map[lib.BlockHash]lib.DeSoSignature) | ||
| for ii, signedInnerTxnHex := range requestData.SignedInnerTransactionsHex { | ||
| // Decode the signed inner transaction. | ||
| signedTxnBytes, err := hex.DecodeString(signedInnerTxnHex) | ||
| if err != nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Problem decoding signed transaction hex: %v", err)) | ||
| return | ||
| } | ||
|
|
||
| // Deserialize the signed transaction. | ||
| signedInnerTxn := &lib.MsgDeSoTxn{} | ||
| if err := signedInnerTxn.FromBytes(signedTxnBytes); err != nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Problem deserializing signed transaction %d from bytes: %v", | ||
| ii, err)) | ||
| return | ||
| } | ||
|
|
||
| // Verify the signature is present. | ||
| if signedInnerTxn.Signature.Sign == nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Signed transaction %d hex missing signature", ii)) | ||
| return | ||
| } | ||
|
|
||
| // Find the pre-signature DeSo transaction hash. | ||
| // NOTE: We do not use the lib.MsgDeSoTxn.Hash() function here as | ||
| // the transactions included in the atomic transaction do not yet | ||
| // have their signature fields set. | ||
| preSignatureInnerTxnBytes, err := signedInnerTxn.ToBytes(true) | ||
| if err != nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Problem serializing "+ | ||
| "signed transaction %d without signature: %v", ii, err)) | ||
| return | ||
| } | ||
| preSignatureInnerTxnHash := lib.Sha256DoubleHash(preSignatureInnerTxnBytes) | ||
| innerTxnPreSignatureHashToSignature[*preSignatureInnerTxnHash] = signedInnerTxn.Signature | ||
| } | ||
|
|
||
| // Based on the provided signatures, complete the atomic transaction. | ||
| for jj, innerTxn := range atomicTxn.TxnMeta.(*lib.AtomicTxnsWrapperMetadata).Txns { | ||
| // Skip signed inner transactions. | ||
| if innerTxn.Signature.Sign != nil { | ||
| continue | ||
| } | ||
|
|
||
| // Find the pre-signature DeSo transaction hash for this transaction. | ||
| preSignatureInnerTxnBytes, err := innerTxn.ToBytes(true) | ||
| if err != nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Problem serializing "+ | ||
| "transaction %d of atomic transaction wrapper without signature: %v", jj, err)) | ||
| return | ||
| } | ||
| preSignatureInnerTxnHash := lib.Sha256DoubleHash(preSignatureInnerTxnBytes) | ||
|
|
||
| // Check that we have the signature. | ||
| if _, exists := innerTxnPreSignatureHashToSignature[*preSignatureInnerTxnHash]; !exists { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Transaction %d in atomic transaction still missing signature", jj)) | ||
| return | ||
| } | ||
|
|
||
| // Set the signature in the atomic transaction. | ||
| atomicTxn.TxnMeta.(*lib.AtomicTxnsWrapperMetadata).Txns[jj].Signature = | ||
| innerTxnPreSignatureHashToSignature[*preSignatureInnerTxnHash] | ||
| } | ||
|
|
||
| // Verify and broadcast the completed atomic transaction. | ||
| if err := fes.backendServer.VerifyAndBroadcastTransaction(atomicTxn); err != nil { | ||
| _AddBadRequestError(ww, | ||
| fmt.Sprintf("SubmitAtomicTransaction: Problem broadcasting transaction: %v", err)) | ||
| return | ||
| } | ||
|
|
||
| res := &SubmitAtomicTransactionResponse{ | ||
| Transaction: atomicTxn, | ||
| TxnHashHex: atomicTxn.Hash().String(), | ||
| TransactionIDBase58Check: lib.PkToString(atomicTxn.Hash()[:], fes.Params), | ||
| } | ||
|
|
||
| if err := json.NewEncoder(ww).Encode(res); err != nil { | ||
| _AddBadRequestError(ww, fmt.Sprintf( | ||
| "SubmitAtomicTransaction: Problem encoding response as JSON: %v", err)) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we move this to a separate PR such that we can accept this change independently of the subsidization changes?
There was a problem hiding this comment.
Yes! I'm going to leave this in until @diamondhands0 has a chance to read through everything.
Something we might consider is leaving this backend PR as a draft until we decide we need it. It shouldn't be too difficult to merge this PR in later if we wanted to. It's mostly just adding new endpoints and helper functions.
| // Grab subsidization keys. | ||
| subsidizationSeedBytes, err := bip39.NewSeedWithErrorChecking(fes.Config.TransactionSubsidizationSeed, "") | ||
| if err != nil { | ||
| _AddBadRequestError(ww, | ||
| fmt.Sprintf("SubsidizedUpdateProfile: Problem generating subsidization seed: %v", err)) | ||
| return | ||
| } | ||
| subsidizationPublicKey, subsidizationPrivateKey, _, err := | ||
| lib.ComputeKeysFromSeed(subsidizationSeedBytes, 0, fes.Params) | ||
| if err != nil { | ||
| _AddBadRequestError(ww, | ||
| fmt.Sprintf("SubsidizedUpdateProfile: Problem computing subsidization keys: %v", err)) | ||
| return | ||
| } |
There was a problem hiding this comment.
I would move all the subsidization logic into its own function. This way we could enforce more strict checks around it and use it more broadly.
There was a problem hiding this comment.
I reviewed this earlier and it's really great code, but I agree with @lazynina that we should keep it as a draft and merge it in later when we need it. At that time we can thoughtfully disable things like the additionalOutputs and guarantee that it's free of any issues.
I would add, though, that it's extremely valuable that you did this full end to end BE<>FE test @poolcoke because without it we would run the risk of having something critical missing in our implementation that would require another hard fork to fix.
| // is equivalent to the sum of the fees paid and the sum of the basic transfers (outputs). | ||
| func ComputeNecessarySubsidyForTransaction(txn *lib.MsgDeSoTxn) uint64 { | ||
| necessarySubsidy := txn.TxnFeeNanos | ||
| for _, txOutput := range txn.TxOutputs { |
No description provided.