Skip to content

ac_private hltc#418

Merged
ca333 merged 31 commits intoGLEECBTC:devfrom
Alrighttt:ARRR-HLTC
May 14, 2021
Merged

ac_private hltc#418
ca333 merged 31 commits intoGLEECBTC:devfrom
Alrighttt:ARRR-HLTC

Conversation

@Alrighttt
Copy link
Copy Markdown

This includes the necessary consensus changes to allow ac_private chains to perform HLTC atomic swaps.

It will check for any p2sh output. If it finds one, it expects the redeemscript for this output to be revealed in the very next output as an OP_RETURN output. It will then validate that this is in fact an HLTC output. This seems a roundabout way of accomplishing this, but this is by far the simplest solution for a few reasons. It allows consensus to continue to hold "Any transparent output can be spent. The forced privacy comes from the inability to create these outputs.". It also allows for using existing mm2 code.

Do not merge this. This does not include the timestamp activation needed to safely introduce these changes.

This includes some debugging code which will not be used in production such as bypassing the check against ac_name to allow additional ac_private chains. There is also a new rpc command added, z_sendmany_template, which allows us to test these changes. It is able to produce unsigned t->z transactions allowing us to spend HLTC inputs to z outputs. I cannot recommend using it in it's current state. If we need a production ready solution this can be adapted. Please let me know.

There is an additional exception to be added at the request of @artemii235 . I will add this exception to this PR tomorrow.

@ca333 @DeckerSU @jl777 @CryptoForge

@ca333 ca333 requested review from DeckerSU, artemii235 and ca333 May 4, 2021 08:14
@Alrighttt
Copy link
Copy Markdown
Author

@artemii235 added your additional script

@DeckerSU
Copy link
Copy Markdown

DeckerSU commented May 5, 2021

@Alrighttt Looks like sources of asyncrpcoperation_sendmany_template.cpp and asyncrpcoperation_sendmany_template.h is absent in this PR? Any reason for that?

@Alrighttt
Copy link
Copy Markdown
Author

Thanks @DeckerSU hadn't noticed. I will create a new branch in my fork to include it in as it doesn't belong in production branches(at least in its current state).

Cleaning up this PR and marking it ready to review later today

@Alrighttt Alrighttt marked this pull request as ready for review May 6, 2021 07:04
@Alrighttt
Copy link
Copy Markdown
Author

Alrighttt commented May 6, 2021

I've added a change to the miner in this PR as a failsafe.

There is an edge case in how this consensus change activates(and generally any other timestamp based activation) if a reorg happens at the exact wrong moment during activation. It's possible for miners to end up with a transaction inside their mempool that violates the current consensus rules. This results in them continually producing bad blocks until they clear out their mempool or the chain's consensus rules finally do update.

The change is important to mitigate an edge case in which all pools on the network would begin producing bad blocks. If they produce a single bad block, they will clear their mempool. This will force any transactions to be revalidated with the correct set of consensus rules.

@DeckerSU

Copy link
Copy Markdown

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

A couple of comments.

@artemii235
Copy link
Copy Markdown

@sergeyboyko0791 As you have great CPP experience could you take a look at it too, please 🙂

Copy link
Copy Markdown

@sergeyboyko0791 sergeyboyko0791 left a comment

Choose a reason for hiding this comment

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

The previous comments are more important than mine :D. Please consider looking at this comment after fixing the previous ones.

Copy link
Copy Markdown

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

1 more comment

@Alrighttt
Copy link
Copy Markdown
Author

I see the issue with the last commit.(accidental merge from the debug branch) Fixing now.

This reverts commit a92eb61, reversing
changes made to 3df6e1b.
Copy link
Copy Markdown

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@ca333 ca333 requested a review from DeckerSU May 13, 2021 17:19
Copy link
Copy Markdown

@DeckerSU DeckerSU left a comment

Choose a reason for hiding this comment

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

LGTM.

Btw, to check script sizes and offset i wrote kind of "simple unit test", may be it will be useful to somebody:

std::cerr << "[ ] IsRedeemScriptReveal emulation tests START" << std::endl;
    
    CScript redeemScript; bool fCheck;
    
    // *** test 1
    redeemScript = CScript() << OP_RETURN << ParseHex("6304e0219b60b1752102cdd0f6e68478c4d904c3ac4a64de132ae0d2470af6d8ce7bfd5aa8ff2c1df6efac6782012088a91460753daf53b427d4dae12f79385c37aa128eede588210301b2324698f3dacd6e076be09f06b58e5462d77e902c3304c078f6d6a367df97ac68");
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;
    fCheck = \
                redeemScript.size() == 110 &&
                redeemScript[0] == OP_RETURN &&
                redeemScript[3] == OP_IF &&
                redeemScript[9] == OP_NOP2 &&
                redeemScript[10] == OP_DROP &&
                redeemScript[45] == OP_CHECKSIG &&
                redeemScript[46] == OP_ELSE &&
                redeemScript[47] == OP_SIZE &&
                redeemScript[48] == 0x01 &&
                redeemScript[49] == 0x20 &&
                redeemScript[50] == OP_EQUALVERIFY &&
                redeemScript[51] == OP_HASH160 &&
                redeemScript[73] == OP_EQUALVERIFY &&
                redeemScript[108] == OP_CHECKSIG &&
                redeemScript[109] == OP_ENDIF;
    std::cerr << "[!] Check: " << (fCheck ? "\033[32mPASSED\033[0m" : "\033[31mFAILED\033[0m") << std::endl;
    redeemScript.erase(redeemScript.begin(),redeemScript.begin()+3 );
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;

    // *** test 2
    redeemScript = CScript() << OP_RETURN << ParseHex("6304b8b79b60b17521030101010101010101010101010101010101010101010101010101010101010101ac6782012088aa2000000000000000000000000000000000000000000000000000000000000000008821030101010101010101010101010101010101010101010101010101010101010101ac68");
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;
    fCheck = \
                redeemScript.size() == 122 &&
                redeemScript[0] == OP_RETURN &&
                redeemScript[3] == OP_IF &&
                redeemScript[9] == OP_NOP2 &&
                redeemScript[10] == OP_DROP &&
                redeemScript[45] == OP_CHECKSIG &&
                redeemScript[46] == OP_ELSE &&
                redeemScript[47] == OP_SIZE &&
                redeemScript[48] == 0x01 &&
                redeemScript[49] == 0x20 &&
                redeemScript[50] == OP_EQUALVERIFY &&
                ( redeemScript[51] == OP_SHA256 || redeemScript[51] == OP_HASH256 ) &&
                redeemScript[85] == OP_EQUALVERIFY &&
                redeemScript[120] == OP_CHECKSIG &&
                redeemScript[121] == OP_ENDIF;
    std::cerr << "[!] Check: " << (fCheck ? "\033[32mPASSED\033[0m" : "\033[31mFAILED\033[0m") << std::endl;
    redeemScript.erase(redeemScript.begin(),redeemScript.begin()+3 );
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;

    // *** test 3
    redeemScript = CScript() << OP_RETURN << ParseHex("10000102030405060708090a0b0c0d0e0f756304e0219b60b1752102aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaac672102bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbac68");
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;
    fCheck = \
                redeemScript.size() == 101 &&
                redeemScript[0] == OP_RETURN &&
                redeemScript[1] == 0x4c && // PUSH_BYTES
                redeemScript[2] == 0x62 && // BYTES
                redeemScript[20] == OP_DROP &&
                redeemScript[21] == OP_IF &&
                redeemScript[22] == 0x04 && // 32bit locktime
                redeemScript[27] == OP_NOP2 &&
                redeemScript[28] == OP_DROP &&
                redeemScript[63] == OP_CHECKSIG &&
                redeemScript[64] == OP_ELSE &&
                redeemScript[99] == OP_CHECKSIG &&
                redeemScript[100] == OP_ENDIF;
    std::cerr << "[!] Check: " << (fCheck ? "\033[32mPASSED\033[0m" : "\033[31mFAILED\033[0m") << std::endl;
    redeemScript.erase(redeemScript.begin(),redeemScript.begin()+3 );
    std::cerr << redeemScript.ToString() << " (" << redeemScript.size() << ")" << std::endl;
    std::cerr << "[ ] IsRedeemScriptReveal emulation tests END" << std::endl;

All checks passed. The only thing that in 3rd case we have more strict check, bcz we verify OP_PUSHDATA1 (0x4C) opcode and size of push bytes, but in 1st and 2nd template case we didn't do such checks. We can add more strict check to all cases, but without - it also looks good.

@ca333 ca333 merged commit 1c34dd9 into GLEECBTC:dev May 14, 2021
DeckerSU added a commit to DeckerSU/KomodoOcean that referenced this pull request May 16, 2021
who-biz pushed a commit to who-biz/komodo that referenced this pull request Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants