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

ICX hardfork fixes #599

Merged
merged 14 commits into from
Jul 9, 2021
Merged

ICX hardfork fixes #599

merged 14 commits into from
Jul 9, 2021

Conversation

Mixa84
Copy link
Contributor

@Mixa84 Mixa84 commented Jul 7, 2021

What kind of PR is this?:

/kind fix

What this PR does / why we need it:

PR contains fixes for the following issues:

  • Not able to manually close offer on BTC/DFI order
  • Trying to refund takerFee when offer expires if atomic swap already started and takerFee burnt. No more message in the log
  • Always recalculating takerFee on HTLC create and leads to negative amount in refund which is not possible
  • Not able to create tx if there is no change due to check for 2 vouts on every ICX tx
  • Several small and rpc fixes

Which issue(s) does this PR fixes?:

Fixes #555
Fixes #583
Fixes #595

Additional comments?:

Fixes will activate on EunosPaya height

@Mixa84 Mixa84 added protocol Has network protocol changes kind/fix labels Jul 7, 2021
@Mixa84 Mixa84 requested a review from a team July 7, 2021 15:39
@Mixa84 Mixa84 self-assigned this Jul 7, 2021
{
CAmount BTCAmount(static_cast<CAmount>((arith_uint256(submitdfchtlc.amount) * arith_uint256(order->orderPrice) / arith_uint256(COIN)).GetLow64()));
takerFee = CalculateTakerFee(BTCAmount);
}

// refund the rest of locked takerFee if there is difference
if (offer->takerFee - takerFee) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this value is negative then still have problem

Copy link
Contributor Author

@Mixa84 Mixa84 Jul 8, 2021

Choose a reason for hiding this comment

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

It will go into else before EunosPaya, and after it will enter the if and calculate the takerFee as ratio of the current one so offer->takerFee - takerFee will never be negative.
We must preserve old functioning before the fork so that we do not create a hardfork before EunosPaya or when syncing from scratch.

surangap
surangap previously approved these changes Jul 8, 2021
@Mixa84 Mixa84 closed this Jul 9, 2021
@Mixa84 Mixa84 reopened this Jul 9, 2021
@Mixa84 Mixa84 merged commit 6012647 into 1.8.x Jul 9, 2021
@Mixa84 Mixa84 deleted the fix/icx_hardfork_fixes branch July 9, 2021 13:56
prasannavl added a commit that referenced this pull request Jul 15, 2021
* v1.8.0

* Remove FortCanning from Eunos regtest setting

* Take anchor block deeper to avoid "Anchor too new" error (#531)

* Add tests for epic/*

* Switch fork to EunosPaya (#584)

* Switch canning to paya 2 (#585)

* Switch fork to EunosPaya

* Remove left over canning changes

* Onchain timelock (#562)

* Prevent resign during PRE_ENABLED state (#518)

* Prevent resign during PRE_ENABLED and staking during PRE_RESIGNED.

* Change FortCanning to EunosPaya

* Reduce future block time to 30 seconds (#586)

* Reduce future time to 30 seconds

* Ignore 30s time rule on regtest

* Check for and reject entries with duplicates (#592)

* Check for and reject duplicates

* Allow duplicates as long as overall sigs meet quorum

* Add tests. Update confirms to store unique on signer ID.

* Restore previous behaviour as collection ignores duplicate CKeyIDs

* Hold confirms on BTC TX height, hash and signed CKeyID (#598)

* Add hard coded seed for devnet (#601)

* Set Eunos and Paya forks (#604)

* ICX hardfork fixes (#599)

* Add check if HTLC existed to skip refund of burnt takerFee

* Fix closing DFI offer not to try to refund amount that is not locked. Fix claimdfchtlc so it cannot be stuck when ext htlc expire.

* Adding EunosPaya height check

* Recalculate takerFee from original takerFee instead from dex

* Fix for ICX txes without change

* Adding EunosPAya height check

* Change fork height check to simplify code

* Fix check on closeoffer

* Fix ExistedHTLC

* Fix listhtlcs rpc params

* Fix passing invalid orderTx in icx_listorders

* Fix error message to be consistent with dfc htlc.

* Fix test

* Fix limit in list RPCs

* Fix uninitialized lastTxOut

* Change the minimum timeout for HTLCs. Offer expires after 20 blocks. Adapt tests. (#611)

* Timelock expiration (#607)

* Timelock expiration

* Pass height to timelock

* Set version to 1.8.0 (#613)

* Set testnet EP fork and increment PROTOCOL_VERSION. (#614)

Co-authored-by: Mihailo Milenkovic <[email protected]>
Co-authored-by: Prasanna Loganathar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol Has network protocol changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants