🐛 Option issue#21

[LesterEvSe]

This PR previously removed changes that limited the number of inputs and outputs due to their inflexibility.

[apoelstra]

3db6ee9 adds a unit test but also weakens the code (I guess because the unit test would be impossible otherwise, and actually the contract is secure?)

For e2621fe see my comments on #21 -- I think (though I didn't try) that you can significantly simplify this code. Having said this, I'm still confused as to why we are working with confidential assets at all. The contract code should have a detailed comment explaining this.

609873b is a merge commit (and a merge commit with nontrivial changes in it).

[LesterEvSe]

3db6ee9 adds a unit test but also weakens the code (I guess because the unit test would be impossible otherwise, and actually the contract is secure?)

Yes, you understood correctly. The reason is described in this comment. This removes the hard coding.

For e2621fe see my comments on #21 -- I think (though I didn't try) that you can significantly simplify this code. Having said this, I'm still confused as to why we are working with confidential assets at all. The contract code should have a detailed comment explaining this.

Okay, after reading roconnor's and your comments, I made some improvements to the code. I also added an explanation of why confidential assets are needed and referred to roconnor's explanation here.

[apoelstra]

In 8166922:

Please change the commit message to remove "feat:" (tests are not features) and to explain that you're crippling the contract by removing the previous mitigation, which was inflexible.

In 6c101e4:

I don't understand how this works. You don't read any data from the input or its issuance. You just take witness data and assert it equal to the output.

[apoelstra]

In 84626ad:

Sorry to keep harping on this, but the commit message really needs to be clearer that it's taking a previously (maybe) secure contract and making it insecure. And that this commit includes a unit test demonstrating the new insecurity, and that it will be patched in a later commit.

In 53ca26f:

This commit significantly rewrites the test introduced in the previous commit. Can you squash the test changes down? It should be that the commit which fixes the security hole simply inverts a test condition, from asserting success to asserting failure.

In d086e50:

This comment looks good to me. Thanks.

[LesterEvSe]

In 84626ad:

Sorry to keep harping on this, but the commit message really needs to be clearer that it's taking a previously (maybe) secure contract and making it insecure. And that this commit includes a unit test demonstrating the new insecurity, and that it will be patched in a later commit.

Okay, not a problem. I will bear these comments in mind in future to help me avoid making the same mistakes. Thank you for the review!