fix: update second_sign method to use schnorr

[galperins4]

Summary

Update the second sign function to use Schnorr

The current code is broken. First sign is Schnorr and second sign was ECSDA which led transactions having issues being verified since they were not correct

Updated a single function in base.py

Checklist

• Documentation (if necessary)
• Tests (if necessary)
• Ready to be merged

[ghost]

Thanks for submitting this pull request! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

[ItsANameToo]

Can you add a test that includes the second signature so this gets caught in any future versions? Just for transfer will suffice for now.

[ItsANameToo]

additionally, it requires a fix in transactions/serializer.py. You'll have to change elif self.transaction.get('signSignature'): to if not skip_second_signature and self.transaction.get('signSignature'):

[ItsANameToo]

requires a secondPublicKey parameter

[ItsANameToo]

After that you can add a tests in tests/transactions/builder/test_transfer.py that is similar to the existing transfer test, but will now have a second passphrase added and verified (for verifying you can use something like transaction.schnorr_verify_second(PublicKey.from_passphrase('this is a top secret passphrase too'), just make sure it matches the passphrase you used to second sign with)

[galperins4]

@ItsANameToo I made all the changes suggested and added the extra test. Feel free to re-review.

[galperins4]

All updated. I think this is ready now.

[ghost]

A member has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait.

Thank you for your contribution!

[ghost]

Your pull request has been merged and marked as tier 4. It will earn you $20 USD.