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
Bitcoin Cash Support #37
Conversation
This exists in bitcoinclassic's code, not sure if that means it does support it:
CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const
{
LOCK(cs);
return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this);
} |
Unfortunately Bitcoin XT supports Bitcoin Classic and Unlimited don't allow the fees to be passed, which may be a problem only if fees rise. |
Just make it great. XT will cherry-pick fundrawtransaction from core. |
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.
thanks for picking up the ball with this. just have some small suggestions about naming
cmd/bccatomicswap/main.go
Outdated
|
||
var ( | ||
flagset = flag.NewFlagSet("", flag.ExitOnError) | ||
connectFlag = flagset.String("s", "localhost", "host[:port] of Bitcoin Core wallet RPC server") |
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.
references bitcoin core
cmd/bccatomicswap/main.go
Outdated
|
||
// There are two directions that the atomic swap can be performed, as the | ||
// initiator can be on either chain. This tool only deals with creating the | ||
// Bitcoin transactions for these swaps. A second tool should be used for the |
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.
references bitcoin
cmd/bccatomicswap/main.go
Outdated
|
||
func init() { | ||
flagset.Usage = func() { | ||
fmt.Println("Usage: btcatomicswap [flags] cmd [cmd args]") |
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.
bccatomicswap (also why is it bccatomicswap and not bchatomicswap?)
cmd/bccatomicswap/main.go
Outdated
|
||
// createSig creates and returns the serialized raw signature and compressed | ||
// pubkey for a transaction input signature. Due to limitations of the Bitcoin | ||
// Core RPC API, this requires dumping a private key and signing in the client, |
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.
references bitcoin core
cmd/bccatomicswap/main.go
Outdated
return fundTransactionViaRPC(rawResp) | ||
} | ||
|
||
func fundTransactionViaRPC(rawResp json.RawMessage) (fundedTx *wire.MsgTx, fee btcutil.Amount, err error) { |
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.
this function name is sort of weird, since the other func above this one is the one actually doing the RPC, and this is just working with a json object.
cmd/bccatomicswap/main.go
Outdated
|
||
fmt.Printf("Secret: %x\n", secret) | ||
fmt.Printf("Secret hash: %x\n\n", secretHash) | ||
fmt.Printf("Contract fee: %v (%0.8f BTC/kB)\n", b.contractFee, contractFeePerKb) |
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.
references bitcoin
cmd/bccatomicswap/main.go
Outdated
contractFeePerKb := calcFeePerKb(b.contractFee, b.contractTx.SerializeSize()) | ||
refundFeePerKb := calcFeePerKb(b.refundFee, b.refundTx.SerializeSize()) | ||
|
||
fmt.Printf("Contract fee: %v (%0.8f BTC/kB)\n", b.contractFee, contractFeePerKb) |
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.
references bitcoin
cmd/bccatomicswap/main.go
Outdated
var buf bytes.Buffer | ||
buf.Grow(redeemTx.SerializeSize()) | ||
redeemTx.Serialize(&buf) | ||
fmt.Printf("Redeem fee: %v (%0.8f BTC/kB)\n\n", fee, redeemFeePerKb) |
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.
references bitcoin
cmd/bccatomicswap/main.go
Outdated
|
||
refundFeePerKb := calcFeePerKb(refundFee, refundTx.SerializeSize()) | ||
|
||
fmt.Printf("Refund fee: %v (%0.8f BTC/kB)\n\n", refundFee, refundFeePerKb) |
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.
references bitcoin
cmd/bccatomicswap/main.go
Outdated
} | ||
|
||
fmt.Printf("Contract address: %v\n", contractAddr) | ||
fmt.Printf("Contract value: %v\n", btcutil.Amount(cmd.contractTx.TxOut[contractOut].Value)) |
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.
this will also reference bitcoin since the amount will be formatted with BTC at the end using the default string conversion
It works. HF hasn't broken it. @jrick One last question: Can this be used to mix coins trustlessly (and privately from others)? Ready To Be Merged! |
no on that last question, you can passively observe both chains and see the swap due to the shared secret once it is revealed by both redeeming parties. |
I'll try to give this another look soon, but it will need to be switched over to use OP_SHA256 instead of OP_RIPEMD160 since we just converted the rest of the tools. See 4c8c7c0 for how to make this change. |
Should I add the new Bitcoin Cash Bech32 address format, or is it too early (14 Jan, planned by Amaury)? Was your sha256 commit tested? Now this line throws an error: https://github.com/DesWurstes/atomicswap/blob/BitcoinCash/cmd/bchatomicswap/main.go#L700 because |
Ugh you're right, it needs updates for txscript as well or else it fails to parse the script as an atomic swap script. Had that sitting in my tree and didn't make those necessary changes first. I'm going to revert the latest commit and put up an issue so that all the maintainers can coordinate, since it will need similar updates to their btcd fork. Thanks for noticing that and sorry for the waste of time. |
Ready To Be Merged
The first Bitcoin Cash atomic swap has been made!
To test both participate and
initiate
, the atomic swap was made as BCC-><-BCC.Proof at the end of this Pull Request.
To do:
dep
extractsecret
, andauditcontract
estimatesmartfee
refund
Proof:
Another proof that I am MCCCS: "I am MCCCS"