-
Notifications
You must be signed in to change notification settings - Fork 92
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
[v0.4.3] staging branch with backports #1586
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This switches the LTC code to using and requiring segwit features. This bumps the asset version to 1. Note that we do not have a good method for supporting multiple asset versions simultaneously, but we can punt on that since LTC has not been deployed on mainnet anywhere to my knowledge.
In handleInit and handleRedeem, do not bother to limit the expiration time of the processInit/processRedeem waiters by the broadcast timeout. Both of the TryFuncs (processInit and processRedeem) will respond in error if the match is revoked, so there is no need to tweak the waiter's expire time. This will prevent a client request timeout when the init or redeem is submitted just before the Swapper revokes the match, when the waiter's expire time could be set to the past despite the inaction checks having not yet revoked the match.
This updates Swapper to proactively revoke matches where any known swap contracts are expired, or in the case of MakerSwapCast where the expected taker contract would be created with a lock time in the past. These scenarios are possible if a transaction takes an extremely long time to reach the required number of confirmations. The new checks are done in checkInactionEventBased, which is called on a ticker. The (*Swapper).failMatch method is given a userFault bool input argument to indicate if the actor should be penalized. matchTracker is given an expiredBy method that returns true if the lock time of either party's *known* swap is before the reference time. Normally this will be called with the present time. checkInactionBlockBased now consults expiredBy when calling failMatch to determine if either of the swap contracts are expired, justifying the inaction. NOTE: client should be updated with similar logic to prevent attempting to make expired contracts or contracts that are about to expire. server/db: option to store forgiven revoked matches When a match is revoked due to no fault of the user, it should be saved in the DB with the forgiven flag set.
If the located contract transaction in processInit is found to have a lock time that is in the past (already expired) revoke the match immediately and respond to the user with an error. No penalties are assesed since processInit first verifies that the proper lock time has been used. This would be the case if the maker's swap took a very long time to reach the required number of confirmations, and when the taker's turn to publish their swap came around it was already passed the expected lock time for their swap (matchTime + 8 hrs). NOTE: client should be updated with similar logic to prevent attempting to make expired contracts or contracts that are about to expire.
…eRater A FeeRater can generate fees when they are not validated by the server, specifically for inputs to Withdraw, PayFee, Send, and possibly Refund. This corresponds to places where (*Core).feeSuggestionAny would be used.
When creating the swap transaction, see if a more current fee rate should be used instead of the prescribed swap rate. This can be particularly helpful for the taker because their swap may be a long time after the match is made because the maker's swap must be fully confirmed first. If the current rate is swapRate < currentRate <= MaxFeeRate, then use the current rate for the swap transaction.
- client/core: Add `EstimateRegistrationTxFee` function in core that determines the tx fee needed to register for the dex - client/asset: Add `EstimateRegistrationTxFee` to wallet interface - client/webserver: Add a new api endpoint `/regtxfee` - ui: Each time the WalletWaitForm is loaded, this endpoint is called to retrieve an estimate
When AuditContract was switched to a hard requirement of txData (the raw tx bytes), it broke certain simnet tests where no server was available to provide the data, and made the asset.Wallet API somewhat incomplete with no way to retrieve this txData in either full node or SPV mode because there is no method to do so. This makes a redeem cli tool or other use case with no server or txdata oracle extremely cumbersome because the raw bytes are required as an input instead of just the coinid/txid. Normally the server should communicate this txData, and the caller can decide to require it. This adds (back) the ability to work with an empty txData as a convenience for recovery tools and testing. This MAY change in the future if a GetTxData method is added for this purpose. The baseWallet AuditContract now attempts retrieval of the transaction output from the network, but it is only ensured to succeed for a full node or, if the tx is confirmed, and SPV wallet. If this is a full node wallet, a simple gettxout RPC is sufficient with no pkScript or "since" time. If this is an SPV wallet, only a confirmed counterparty contract can be located, and only one within the last 48 hours. As such, this mode of operation is not intended for normal server-coordinate operation. The above change fixes the "livetest" package, which is also used by all btc "clone" assets. The test code is also updated with some extra sleeps for SPV wallets. A comment at the call site of AuditContract is made noting that a nil txData must be provided because in normal operation that data comes from the server coordinating the swap, and that the Wallet API has no ability to get this data.
Add the Onion config field, for use with .onion DEX hosts. If Onion is not set, the TorProxy value is used instead. connectDEX recognizes .onion hosts and uses the Onion proxy. In connectDEX, the scheme (ws or wss) is determined by the presence of a TLS certificate. Only .onion hosts are permitted to use a no-TLS connection ("ws" scheme). parseCert is updated so that a nil certI interface{} is permitted, in which case it checks the cert store, but a nil []byte may be expected if the host is a .onion service.
martonp
approved these changes
Apr 17, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to register fine with DCR. BTC fee suggestion looks good as well, but I'm having trouble finding enough testnet BTC to register.
Thanks for testing. Reg with both is working for me too. Ping me on matrix regarding testnet btc |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The intent is to fast forward
release-v0.4
to this.These are the closed tems in https://github.com/decred/dcrdex/milestone/19?closed=1
The dex-test server has been running this.
@martonp and @buck54321, of note are the FeeRater interface change and the registration tx fee estimates that it facilitates. These changes narrowly missed the last patch releases, but the reg fee is such a big UX headache that we need it in 0.4.3. As such, we should test some fresh registrations with BTC and DCR as the fee asset with this branch. The rest of the commits are well tested, but please ACK regardless.
After this backport PR the remaining 0.4.3 changes are:
My remote/fork has 0.4 branches for the above, where the p2pk fix is included in my doge-0.4 testing branch.