Good questions/ideas here! I agree with you that
ApprovedPayment is cleaner and more flexible. However, the problem is that we must have dual attestation on all transactions between VASPs over a certain amount–there can’t be any loopholes. Because
ApprovedPayment is opt-in, it doesn’t satisfy this requirement.
Now let me try to reply to some of your individual questions:
I cannot have a VASP account in testnet
I don’t know the private key for the hardcoded default vasp account https://github.com/libra/libra/blob/testnet/language/stdlib/modules/libra_account.move#L400 and cannot use that for testing
All testnet accounts are now VASP accounts (though this is a fairly recent change). Although the compliance key starts off as a dummy key, you can rotate it to something you know using this code. That will allow you to test the dual attestation functionality.
I think it would be good to allow the approved transaction for any account not just vasps, because I think it would be a pretty useful feature to be able to make sure the correct amount is paid. If you for example create a bill for some amount, with this feature you can validate that the correct amount will be paid.
I think this is a great idea and would be very easy to implement by tweaking the existing scheme–we can simply opt in to doing the check whenever the
signature parameter is nonempty.
Separating this feature into another transaction script would make this cleaner. Now the script can only partly be used by a normal account and the part that cannot be used just makes it confusing.
I agree with this. We are planning to add simpler transaction scripts for payments without dual attestation (to eliminate the
signature parameter) and for payments that cannot create new accounts (to eliminate the
auth_key_prefix parameter). We are currently in a confusing middle ground between the old, simpler testnet and the new one with VASPs etc.
The signing process of the metadata is not similar to the one used for a transaction. The data is not hashed before adding salt and signing. I don’t see why the signing has to be different for this.
Agreed that consistency would be valuable here. We are actually working on changing the transaction signing to sign the raw bytes.
-Also, the salt / domain separator is very different from the one used for signing a transaction libra_types::transaction::RawTransaction@@$$LIBRA$$@@ vs @LIBRA_ATTEST@
This is intentional. The purpose of domain separators is to use a distinct separator for each kind of message (to avoid, e.g., signing dual attestation bytes and then interpreting them as a signature on a transaction).
Consider adding a structure containing all the fields that are required for the signature. Then the signing would be pretty close to signing a transaction: just serialize the structure using lcs, create a domain separator the same way it’s done in transactions and sign it using the same process.
We considered this, but felt that the flat bytearray was simpler to implement for clients (fewer LCS concepts to implement/worry about).