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

Add Particl Support #29

Merged
merged 1 commit into from Oct 24, 2017
Merged

Add Particl Support #29

merged 1 commit into from Oct 24, 2017

Conversation

dasource
Copy link
Contributor

@dasource dasource commented Oct 5, 2017

This implements the partatomicswap command.
Happy to run some tests with you.

Thank for the great work!

@dajohi
Copy link
Member

dajohi commented Oct 6, 2017

Rebase?

@jrick
Copy link
Member

jrick commented Oct 6, 2017

Does particl have a running testnet network? (only asking because some alts seem to not bother running one)

@dasource
Copy link
Contributor Author

dasource commented Oct 6, 2017

@jrick sure do.
You can also see the testnet explorer here: https://explorer-testnet.particl.io

@jrick
Copy link
Member

jrick commented Oct 12, 2017

Seems to work well with the exception of the fee estimation. I noticed that when the estimatefee RPC failed (probably because my node had not been online for 6 blocks) that the contract transaction was created with a low fee. This was accepted on testnet but I imagine it would be rejected on mainnet due to standard checks.

For example:

Contract fee: 0.00000171 PART (0.00000753 PART/kB)

Which is lower than the mempool fee rate policy than I see from getinfo:

  "relayfee": 0.00001000,

The refund and redemption transaction fee estimation appeared to work fine (the fee rate was never lower than 1e3).

@jrick
Copy link
Member

jrick commented Oct 12, 2017

Actually on further inspection, I don't seem to be getting any valid results out of the estimatefee RPC using particl-cli. Should this part of the getFeePerKb func be removed?

@dasource
Copy link
Contributor Author

Thanks @jrick for testing this.

  1. estimatefee is deprecated in 0.15.0.x and should be replaced with estimatesmartfee.
  2. in either case both where returning -1 during your tests due to lack of TXs across testnet (was fine on mainnet) since your node came online. Once we sent some TXs on testnet this started returning correct data.

Thanks again for the initiative on this and helping to test it.

@jrick
Copy link
Member

jrick commented Oct 13, 2017

After discussion with @dasource, the estimatefee RPC does work but was failing to produce any estimates on testnet due to low transaction activity at the time. I have confirmed that estimatefee does produce results after testnet was spammed with transactions.

This should be using estimatesmartfee, not estimatefee. This is being tracked in #32 and I can either update it later or partatomicswap can be the first of the tools to use this RPC.

The issues with the contract transaction having a low fee rate is due to the fee being applied by fundrawtransaction. I will need to do more testing against Bitcoin Core's 0.15 branch to see if this applies to all of the non-DCR tools or is a bug with particld.

@jrick
Copy link
Member

jrick commented Oct 23, 2017

I tested against bitcoin core 0.15 and the contract is created with the correct fee, so this does appear to be a bug with particld. I expect contract transactions to be rejected on mainnet due to low fees (but have not tested this, only tested on testnet so far).

@tecnovert tecnovert force-pushed the master branch 2 times, most recently from 44a93c8 to 9b0f2fb Compare October 24, 2017 15:38
@jrick jrick merged commit 2bdb0d8 into decred:master Oct 24, 2017
@jrick jrick moved this from Currencies being added to Completed in Additional cryptocurrency interop support Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants