diff --git a/Cargo.lock b/Cargo.lock index a7574a9ff6..dc5e5cb17c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7863,6 +7863,7 @@ dependencies = [ "hex", "lazy_static", "log", + "pallet-balances", "pallet-collective", "pallet-handles", "pallet-schemas", diff --git a/e2e/msa/msaTokens.test.ts b/e2e/msa/msaTokens.test.ts index 60605affe9..728d3f4269 100644 --- a/e2e/msa/msaTokens.test.ts +++ b/e2e/msa/msaTokens.test.ts @@ -1,15 +1,32 @@ import '@frequency-chain/api-augment'; import assert from 'assert'; -import { ExtrinsicHelper } from '../scaffolding/extrinsicHelpers'; -import { ethereumAddressToKeyringPair } from '@frequency-chain/ethereum-utils'; +import { AuthorizedKeyData, ExtrinsicHelper } from '../scaffolding/extrinsicHelpers'; +import { + EcdsaSignature, + createAuthorizedKeyData, + ethereumAddressToKeyringPair, + getUnifiedAddress, + getUnifiedPublicKey, + signEip712, +} from '@frequency-chain/ethereum-utils'; import { getFundingSource } from '../scaffolding/funding'; import { H160 } from '@polkadot/types/interfaces'; -import { bnToU8a, hexToU8a, stringToU8a } from '@polkadot/util'; +import { bnToU8a, hexToU8a, stringToU8a, u8aToHex } from '@polkadot/util'; import { KeyringPair } from '@polkadot/keyring/types'; import { keccak256AsU8a } from '@polkadot/util-crypto'; -import { getExistentialDeposit } from '../scaffolding/helpers'; +import { + CENTS, + createAndFundKeypair, + createKeys, + DOLLARS, + generateAuthorizedKeyPayload, + getEthereumKeyPairFromUnifiedAddress, +} from '../scaffolding/helpers'; +import { u64 } from '@polkadot/types'; +import { Codec } from '@polkadot/types/types'; const fundingSource = getFundingSource(import.meta.url); +const TRANSFER_AMOUNT = 1n * DOLLARS; /** * @@ -37,6 +54,23 @@ function generateMsaAddress(msaId: string | number | bigint): H160 { return ExtrinsicHelper.api.registry.createType('H160', hash.slice(-20)); } +async function generateSignedAuthorizedKeyPayload(keys: KeyringPair, payload: AuthorizedKeyData) { + const signingPayload = createAuthorizedKeyData( + payload.msaId!.toString(), + u8aToHex(payload.authorizedPublicKey), + payload.expiration + ); + const ownerSig = await signEip712( + u8aToHex(getEthereumKeyPairFromUnifiedAddress(getUnifiedAddress(keys)).secretKey), + signingPayload + ); + + return { + signingPayload, + ownerSig, + }; +} + describe('MSAs Holding Tokens', function () { const MSA_ID_1234 = 1234; // MSA ID for testing const CHECKSUMMED_ETH_ADDR_1234 = '0x65928b9a88Db189Eea76F72d86128Af834d64c32'; // Checksummed Ethereum address for MSA ID 1234 @@ -80,14 +114,12 @@ describe('MSAs Holding Tokens', function () { describe('Send tokens to MSA', function () { it('should send tokens to the MSA', async function () { - const ed = await getExistentialDeposit(); - const transferAmount = 1n + ed; let accountData = await ExtrinsicHelper.getAccountInfo(ethKeys); const initialBalance = accountData.data.free.toBigInt(); const op = ExtrinsicHelper.transferFunds( fundingSource, ethereumAddressToKeyringPair(ethAddress20), - transferAmount + TRANSFER_AMOUNT ); const { target: transferEvent } = await op.fundAndSend(fundingSource); @@ -97,9 +129,207 @@ describe('MSAs Holding Tokens', function () { const finalBalance = accountData.data.free.toBigInt(); assert.equal( finalBalance, - initialBalance + transferAmount, + initialBalance + TRANSFER_AMOUNT, 'Final balance should be increased by transfer amount' ); }); }); + + describe('withdrawTokens', function () { + let msaKeys: KeyringPair; + let msaId: u64; + let msaAddress: H160; + let otherMsaKeys: KeyringPair; + let secondaryKey: KeyringPair; + let defaultPayload: AuthorizedKeyData; + let payload: AuthorizedKeyData; + let ownerSig: EcdsaSignature; + let badSig: EcdsaSignature; + + before(async function () { + // Setup an MSA with tokens + msaKeys = await createAndFundKeypair(fundingSource, 5n * CENTS, undefined, undefined, 'ethereum'); + let { target } = await ExtrinsicHelper.createMsa(msaKeys).signAndSend(); + assert.notEqual(target?.data.msaId, undefined, 'MSA Id not in expected event'); + msaId = target!.data.msaId; + + // Setup another MSA control key + otherMsaKeys = await createAndFundKeypair(fundingSource, 5n * CENTS, undefined, undefined, 'ethereum'); + ({ target } = await ExtrinsicHelper.createMsa(otherMsaKeys).signAndSend()); + assert.notEqual(target?.data.msaId, undefined, 'MSA Id not in expected event'); + + const { accountId } = await ExtrinsicHelper.apiPromise.call.msaRuntimeApi.getEthereumAddressForMsaId(msaId); + msaAddress = accountId; + + // Create unfunded keys; this extrinsic should be free + secondaryKey = createKeys(undefined, 'ethereum'); + + // Default payload making it easier to test `withdrawTokens` + defaultPayload = { + msaId, + authorizedPublicKey: getUnifiedPublicKey(secondaryKey), + }; + }); + + beforeEach(async function () { + payload = await generateAuthorizedKeyPayload(defaultPayload); + }); + + it('should fail if origin is not address contained in the payload (NotKeyOwner)', async function () { + const badPayload = { ...payload, authorizedPublicKey: getUnifiedAddress(createKeys()) }; // Invalid MSA ID + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, payload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, badPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 5', // NotKeyOwner, + }); + }); + + it('should fail if MSA owner signature is invalid (MsaOwnershipInvalidSignature)', async function () { + ({ ownerSig: badSig } = await generateSignedAuthorizedKeyPayload(createKeys('badKeys', 'ethereum'), payload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, badSig, payload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 8', // MsaOwnershipInvalidSignature + }); + }); + + it('should fail if expiration has passed (MsaOwnershipInvalidSignature)', async function () { + const newPayload = await generateAuthorizedKeyPayload({ + ...defaultPayload, + expiration: (await ExtrinsicHelper.getLastBlock()).block.header.number.toNumber(), + }); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, newPayload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, newPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 8', // MsaOwnershipInvalidSignature, + }); + }); + + it('should fail if expiration is not yet valid (MsaOwnershipInvalidSignature)', async function () { + const maxMortality = ExtrinsicHelper.api.consts.msa.mortalityWindowSize.toNumber(); + const newPayload = await generateAuthorizedKeyPayload({ + ...defaultPayload, + expiration: (await ExtrinsicHelper.getLastBlock()).block.header.number.toNumber() + maxMortality + 999, + }); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, newPayload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, newPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 8', // MsaOwnershipInvalidSignature, + }); + }); + + it('should fail if origin is an MSA control key (IneligibleOrigin)', async function () { + const newPayload = await generateAuthorizedKeyPayload({ + ...defaultPayload, + authorizedPublicKey: getUnifiedPublicKey(otherMsaKeys), + }); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, newPayload)); + const op = ExtrinsicHelper.withdrawTokens(otherMsaKeys, msaKeys, ownerSig, newPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 10', // IneligibleOrigin, + }); + }); + + it('should fail if payload signer does not control the MSA in the signed payload (InvalidMsaKey)', async function () { + const newPayload = await generateAuthorizedKeyPayload({ + ...defaultPayload, + msaId: new u64(ExtrinsicHelper.api.registry, 9999), + }); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, newPayload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, newPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 1', // InvalidMsaKey, + }); + }); + + it('should fail if payload signer is not an MSA control key (InvalidMsaKey)', async function () { + const badSigner = createKeys(undefined, 'ethereum'); + const newPayload = await generateAuthorizedKeyPayload({ + ...defaultPayload, + msaId: new u64(ExtrinsicHelper.api.registry, 9999), + }); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(badSigner, newPayload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, badSigner, ownerSig, newPayload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 1', // InvalidMsaKey, + }); + }); + + it('should fail if MSA does not have a balance', async function () { + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, payload)); + const op = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, payload); + await assert.rejects(op.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 9', // InsufficientBalanceToWithdraw, + }); + }); + + it('should succeed', async function () { + const { + data: { free: startingBalance }, + } = await ExtrinsicHelper.getAccountInfo(secondaryKey); + // Send tokens to MSA + const op1 = ExtrinsicHelper.transferFunds( + fundingSource, + ethereumAddressToKeyringPair(msaAddress), + TRANSFER_AMOUNT + ); + await assert.doesNotReject(op1.signAndSend(), 'MSA funding failed'); + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, payload)); + const op2 = ExtrinsicHelper.withdrawTokens(secondaryKey, msaKeys, ownerSig, payload); + await assert.doesNotReject(op2.signAndSend('current'), 'token transfer transaction should have succeeded'); + // Destination account should have had balance increased + const { + data: { free: endingBalance }, + } = await ExtrinsicHelper.getAccountInfo(secondaryKey); + + assert( + startingBalance.toBigInt() + TRANSFER_AMOUNT === endingBalance.toBigInt(), + 'balance of recieve should have increased by the transfer amount minus fee' + ); + }); + + it('should fail for duplicate signature submission (MsaOwnershipInvalidSignature)', async function () { + // In order to test this, we need to create a new keypair and fund it, because otherwise the nonce will + // be the same for both transactions (and, because we're using Edcs signatures, the signature will be the same). + // Not sure exactly what happens in this case, but it seems to be that the second transaction is siliently dropped + // by the node, but the status call back in polkadot.js still resolves (ie, gets 'isInBlock' or 'isFinalized') + const keys = await createAndFundKeypair(fundingSource, 5n * CENTS, undefined, undefined, 'ethereum'); + payload.authorizedPublicKey = getUnifiedPublicKey(keys); + + const op1 = ExtrinsicHelper.transferFunds( + fundingSource, + ethereumAddressToKeyringPair(msaAddress), + TRANSFER_AMOUNT + ); + await assert.doesNotReject(op1.signAndSend(), 'MSA funding failed'); + + ({ ownerSig } = await generateSignedAuthorizedKeyPayload(msaKeys, payload)); + let op2 = ExtrinsicHelper.withdrawTokens(keys, msaKeys, ownerSig, payload); + await assert.doesNotReject(op2.signAndSend('current'), 'token withdrawal should have succeeded'); + + // Re-fund MSA so we don't fail for that + await assert.doesNotReject(op1.signAndSend(), 'MSA re-funding failed'); + op2 = ExtrinsicHelper.withdrawTokens(keys, msaKeys, ownerSig, payload); + await assert.rejects(op2.signAndSend('current'), { + name: 'RpcError', + code: 1010, + data: 'Custom error: 8', // MsaOwnershipInvalidSignature, + }); + }); + }); }); diff --git a/e2e/scaffolding/extrinsicHelpers.ts b/e2e/scaffolding/extrinsicHelpers.ts index 56a3e93553..7d9af61278 100644 --- a/e2e/scaffolding/extrinsicHelpers.ts +++ b/e2e/scaffolding/extrinsicHelpers.ts @@ -4,12 +4,7 @@ import { ApiPromise, ApiRx } from '@polkadot/api'; import { ApiTypes, AugmentedEvent, SubmittableExtrinsic, SignerOptions } from '@polkadot/api/types'; import { KeyringPair } from '@polkadot/keyring/types'; import { Compact, u128, u16, u32, u64, Vec, Option, Bool } from '@polkadot/types'; -import { - FrameSystemAccountInfo, - PalletTimeReleaseReleaseSchedule, - SpRuntimeDispatchError, - PalletSchedulerScheduled, -} from '@polkadot/types/lookup'; +import { FrameSystemAccountInfo, SpRuntimeDispatchError } from '@polkadot/types/lookup'; import { AnyJson, AnyNumber, AnyTuple, Codec, IEvent, ISubmittableResult } from '@polkadot/types/types'; import { firstValueFrom, filter, map, pipe, tap } from 'rxjs'; import { getBlockNumber, getExistentialDeposit, getFinalizedBlockNumber, log, MultiSignatureType } from './helpers'; @@ -31,6 +26,7 @@ import { u8aWrapBytes } from '@polkadot/util'; import type { AccountId32, Call, H256 } from '@polkadot/types/interfaces/runtime'; import { hasRelayChain } from './env'; import { getUnifiedAddress, getUnifiedPublicKey } from '@frequency-chain/ethereum-utils'; +import { RpcErrorInterface } from '@polkadot/rpc-provider/types'; export interface ReleaseSchedule { start: number; @@ -44,6 +40,11 @@ export interface AddKeyData { expiration?: any; newPublicKey?: any; } +export interface AuthorizedKeyData { + msaId: u64; + expiration?: number | any; + authorizedPublicKey: KeyringPair['publicKey']; +} export interface AddProviderPayload { authorizedMsaId?: u64; schemaIds?: u16[]; @@ -91,6 +92,10 @@ export interface PaginatedDeleteSignaturePayloadV2 { expiration?: any; } +export function isRpcError(e: any): e is RpcErrorInterface { + return e?.name === 'RpcError'; +} + export class EventError extends Error { name: string = ''; message: string = ''; @@ -188,6 +193,7 @@ export class Extrinsic { @@ -208,8 +214,11 @@ export class Extrinsic ExtrinsicHelper.api.tx.msa.withdrawTokens(getUnifiedPublicKey(ownerKeys), ownerSignature, payload), + keys + ); + } } diff --git a/e2e/scaffolding/helpers.ts b/e2e/scaffolding/helpers.ts index 0268e0cc73..6f6b4abb93 100644 --- a/e2e/scaffolding/helpers.ts +++ b/e2e/scaffolding/helpers.ts @@ -16,6 +16,7 @@ import { import { AddKeyData, AddProviderPayload, + AuthorizedKeyData, EventMap, ExtrinsicHelper, ItemizedSignaturePayloadV2, @@ -120,7 +121,7 @@ export async function getBlockNumber(): Promise { let cacheED: null | bigint = null; -export async function getExistentialDeposit(): Promise { +export function getExistentialDeposit(): bigint { if (cacheED !== null) return cacheED; return (cacheED = ExtrinsicHelper.api.consts.balances.existentialDeposit.toBigInt()); } @@ -138,6 +139,18 @@ export async function generateAddKeyPayload( }; } +export async function generateAuthorizedKeyPayload( + payloadInputs: AuthorizedKeyData, + expirationOffset: number = 100, + blockNumber?: number +): Promise { + const { expiration, ...payload } = payloadInputs; + return { + expiration: expiration || (blockNumber || (await getBlockNumber())) + expirationOffset, + ...payload, + }; +} + export async function generateItemizedSignaturePayload( payloadInputs: ItemizedSignaturePayloadV2, expirationOffset: number = 100, diff --git a/js/ethereum-utils/src/payloads.ts b/js/ethereum-utils/src/payloads.ts index 8408074888..6f36a63767 100644 --- a/js/ethereum-utils/src/payloads.ts +++ b/js/ethereum-utils/src/payloads.ts @@ -115,6 +115,17 @@ export interface AddKeyData { newPublicKey: HexString; } +export interface AuthorizedKeyData { + // type discriminator + type: 'AuthorizedKeyData'; + // uint64 type MessageSourceId + msaId: string; + // uint32 type payload expiration block number + expiration: number; + // hex encoded public key to be signed + authorizedPublicKey: HexString; +} + export interface AddProvider { // type discriminator type: 'AddProvider'; @@ -133,11 +144,16 @@ export type SupportedPayload = | PasskeyPublicKey | ClaimHandlePayload | AddKeyData + | AuthorizedKeyData | AddProvider; +export type NormalizedSupportedPayload = Omit; + export interface EipDomainPayload { name: string; version: string; chainId: HexString; verifyingContract: HexString; } + +export type SupportedPayloadTypes = SupportedPayload['type']; diff --git a/js/ethereum-utils/src/signature.definitions.ts b/js/ethereum-utils/src/signature.definitions.ts index f17f005f42..a24ce13d11 100644 --- a/js/ethereum-utils/src/signature.definitions.ts +++ b/js/ethereum-utils/src/signature.definitions.ts @@ -63,6 +63,23 @@ export const ADD_KEY_DATA_DEFINITION = { ], }; +export const AUTHORIZED_KEY_DATA_DEFINITION = { + AuthorizedKeyData: [ + { + name: 'msaId', + type: 'uint64', + }, + { + name: 'expiration', + type: 'uint32', + }, + { + name: 'authorizedPublicKey', + type: 'address', + }, + ], +}; + export const CLAIM_HANDLE_PAYLOAD_DEFINITION = { ClaimHandlePayload: [ { @@ -156,3 +173,16 @@ export const ITEMIZED_SIGNATURE_PAYLOAD_DEFINITION_V2 = { { name: 'index', type: 'uint16' }, ], }; + +const PAYLOAD_DEFINITIONS = [ + ADD_PROVIDER_DEFINITION, + ADD_KEY_DATA_DEFINITION, + AUTHORIZED_KEY_DATA_DEFINITION, + CLAIM_HANDLE_PAYLOAD_DEFINITION, + PASSKEY_PUBLIC_KEY_DEFINITION, + PAGINATED_DELETE_SIGNATURE_PAYLOAD_DEFINITION_V2, + PAGINATED_UPSERT_SIGNATURE_PAYLOAD_DEFINITION_V2, + ITEMIZED_SIGNATURE_PAYLOAD_DEFINITION_V2, +]; + +export type SupportedPayloadDefinitions = (typeof PAYLOAD_DEFINITIONS)[number]; diff --git a/js/ethereum-utils/src/signature.ts b/js/ethereum-utils/src/signature.ts index ed84d86212..360e0f87df 100644 --- a/js/ethereum-utils/src/signature.ts +++ b/js/ethereum-utils/src/signature.ts @@ -1,5 +1,6 @@ import { AddKeyData, + AuthorizedKeyData, AddProvider, ChainType, ClaimHandlePayload, @@ -14,14 +15,17 @@ import { DeleteItemizedAction, ItemizedAction, EipDomainPayload, + NormalizedSupportedPayload, + SupportedPayloadTypes, } from './payloads'; import { assert, isHexString, isValidUint16, isValidUint32, isValidUint64String } from './utils'; import { reverseUnifiedAddressToEthereumAddress } from './address'; -import { ethers, TypedDataField } from 'ethers'; +import { ethers } from 'ethers'; import { u8aToHex } from '@polkadot/util'; import { ADD_KEY_DATA_DEFINITION, ADD_PROVIDER_DEFINITION, + AUTHORIZED_KEY_DATA_DEFINITION, CLAIM_HANDLE_PAYLOAD_DEFINITION, EIP712_DOMAIN_DEFAULT, EIP712_DOMAIN_DEFINITION, @@ -29,6 +33,7 @@ import { PAGINATED_DELETE_SIGNATURE_PAYLOAD_DEFINITION_V2, PAGINATED_UPSERT_SIGNATURE_PAYLOAD_DEFINITION_V2, PASSKEY_PUBLIC_KEY_DEFINITION, + SupportedPayloadDefinitions, } from './signature.definitions'; /** @@ -72,8 +77,8 @@ export function verifyEip712Signature( return recoveredAddress.toLowerCase() === ethereumAddress.toLowerCase(); } -function normalizePayload(payload: SupportedPayload): Record { - const clonedPayload = Object.assign({}, payload); +function normalizePayload(payload: SupportedPayload): NormalizedSupportedPayload { + const clonedPayload: typeof payload = Object.assign({}, payload); switch (clonedPayload.type) { case 'PaginatedUpsertSignaturePayloadV2': case 'PaginatedDeleteSignaturePayloadV2': @@ -91,6 +96,16 @@ function normalizePayload(payload: SupportedPayload): Record { clonedPayload.newPublicKey = clonedPayload.newPublicKey.toLowerCase() as HexString; break; + case 'AuthorizedKeyData': + // convert to 20 bytes ethereum address for signature + if (clonedPayload.authorizedPublicKey.length !== 42) { + clonedPayload.authorizedPublicKey = reverseUnifiedAddressToEthereumAddress( + (payload as AuthorizedKeyData).authorizedPublicKey + ); + } + clonedPayload.authorizedPublicKey = clonedPayload.authorizedPublicKey.toLowerCase() as HexString; + break; + default: throw new Error(`Unsupported payload type: ${JSON.stringify(payload)}`); } @@ -101,14 +116,15 @@ function normalizePayload(payload: SupportedPayload): Record { return payloadWithoutType; } -function getTypesFor(payloadType: string): Record { - const PAYLOAD_TYPE_DEFINITIONS: Record> = { +function getTypesFor(payloadType: string): SupportedPayloadDefinitions { + const PAYLOAD_TYPE_DEFINITIONS: Record = { PaginatedUpsertSignaturePayloadV2: PAGINATED_UPSERT_SIGNATURE_PAYLOAD_DEFINITION_V2, PaginatedDeleteSignaturePayloadV2: PAGINATED_DELETE_SIGNATURE_PAYLOAD_DEFINITION_V2, ItemizedSignaturePayloadV2: ITEMIZED_SIGNATURE_PAYLOAD_DEFINITION_V2, PasskeyPublicKey: PASSKEY_PUBLIC_KEY_DEFINITION, ClaimHandlePayload: CLAIM_HANDLE_PAYLOAD_DEFINITION, AddKeyData: ADD_KEY_DATA_DEFINITION, + AuthorizedKeyData: AUTHORIZED_KEY_DATA_DEFINITION, AddProvider: ADD_PROVIDER_DEFINITION, }; @@ -147,6 +163,32 @@ export function createAddKeyData( }; } +/** + * Build an AuthorizedKeyData payload for signature. + * + * @param msaId MSA ID (uint64) to add the key + * @param authorizedPublicKey 32 bytes public key to authorize in hex or Uint8Array + * @param expirationBlock Block number after which this payload is invalid + */ +export function createAuthorizedKeyData( + msaId: string | bigint, + newPublicKey: HexString | Uint8Array, + expirationBlock: number +): AuthorizedKeyData { + const parsedMsaId: string = typeof msaId === 'string' ? msaId : `${msaId}`; + const parsedNewPublicKey: HexString = typeof newPublicKey === 'object' ? u8aToHex(newPublicKey) : newPublicKey; + + assert(isValidUint64String(parsedMsaId), 'msaId should be a valid uint64'); + assert(isValidUint32(expirationBlock), 'expiration should be a valid uint32'); + assert(isHexString(parsedNewPublicKey), 'newPublicKey should be valid hex'); + return { + type: 'AuthorizedKeyData', + msaId: parsedMsaId, + expiration: expirationBlock, + authorizedPublicKey: parsedNewPublicKey, + }; +} + /** * Build an AddProvider payload for signature. * @@ -326,6 +368,25 @@ export function getEip712BrowserRequestAddKeyData( return createEip712Payload(ADD_KEY_DATA_DEFINITION, message.type, domain, normalized); } +/** + * Returns the EIP-712 browser request for a AuthorizedKeyData for signing. + * + * @param msaId MSA ID (uint64) to add the key + * @param authorizedPublicKey 32 bytes public key to add in hex or Uint8Array + * @param expirationBlock Block number after which this payload is invalid + * @param domain + */ +export function getEip712BrowserRequestAuthorizedKeyData( + msaId: string | bigint, + authorizedPublicKey: HexString | Uint8Array, + expirationBlock: number, + domain: EipDomainPayload = EIP712_DOMAIN_DEFAULT +): unknown { + const message = createAuthorizedKeyData(msaId, authorizedPublicKey, expirationBlock); + const normalized = normalizePayload(message); + return createEip712Payload(AUTHORIZED_KEY_DATA_DEFINITION, message.type, domain, normalized); +} + /** * Returns the EIP-712 browser request for a AddProvider for signing. * @@ -442,7 +503,12 @@ export function getEip712BrowserRequestPasskeyPublicKey( return createEip712Payload(PASSKEY_PUBLIC_KEY_DEFINITION, message.type, domain, normalized); } -function createEip712Payload(typeDefinition: any, primaryType: any, domain: EipDomainPayload, message: any): unknown { +function createEip712Payload( + typeDefinition: SupportedPayloadDefinitions, + primaryType: SupportedPayloadTypes, + domain: EipDomainPayload, + message: NormalizedSupportedPayload +): unknown { return { types: { ...EIP712_DOMAIN_DEFINITION, diff --git a/pallets/capacity/src/tests/mock.rs b/pallets/capacity/src/tests/mock.rs index 611e140c41..c7d422c67d 100644 --- a/pallets/capacity/src/tests/mock.rs +++ b/pallets/capacity/src/tests/mock.rs @@ -143,6 +143,7 @@ impl pallet_msa::Config for Test { type CreateProviderViaGovernanceOrigin = EnsureSigned; /// This MUST ALWAYS be MaxSignaturesPerBucket * NumberOfBuckets. type MaxSignaturesStored = ConstU32<8000>; + type Currency = pallet_balances::Pallet; } // not used yet diff --git a/pallets/frequency-tx-payment/src/tests/mock.rs b/pallets/frequency-tx-payment/src/tests/mock.rs index 85f6424fc2..4ca565ea27 100644 --- a/pallets/frequency-tx-payment/src/tests/mock.rs +++ b/pallets/frequency-tx-payment/src/tests/mock.rs @@ -169,6 +169,7 @@ impl pallet_msa::Config for Test { type CreateProviderViaGovernanceOrigin = EnsureSigned; /// This MUST ALWAYS be MaxSignaturesPerBucket * NumberOfBuckets. type MaxSignaturesStored = ConstU32<8000>; + type Currency = pallet_balances::Pallet; } // Needs parameter_types! for the impls below diff --git a/pallets/msa/Cargo.toml b/pallets/msa/Cargo.toml index 399764e89d..1dbd8e2ca7 100644 --- a/pallets/msa/Cargo.toml +++ b/pallets/msa/Cargo.toml @@ -33,6 +33,7 @@ hex = { workspace = true, default-features = false, features = ["alloc"] } common-runtime = { path = "../../runtime/common", default-features = false } [dev-dependencies] +pallet-balances = { workspace = true } pallet-schemas = { path = "../schemas", default-features = false } pallet-handles = { path = "../handles", default-features = false } pallet-collective = { workspace = true } diff --git a/pallets/msa/README.md b/pallets/msa/README.md index 3361828911..20c3133c1b 100644 --- a/pallets/msa/README.md +++ b/pallets/msa/README.md @@ -49,6 +49,7 @@ The MSA pallet provides for: | `retire_msa`
Remove all keys and mark the MSA as retired | Delegator | Free | [`PublicKeyDeleted`](https://frequency-chain.github.io/frequency/pallet_msa/pallet/enum.Event.html#variant.PublicKeyDeleted), [`MsaRetired`](https://frequency-chain.github.io/frequency/pallet_msa/pallet/enum.Event.html#variant.MsaRetired) | 18 | | `revoke_delegation_by_delegator`
Remove delegation | Delegator | Free | [`DelegationRevoked`](https://frequency-chain.github.io/frequency/pallet_msa/pallet/enum.Event.html#variant.DelegationRevoked) | 1 | | `revoke_delegation_by_provider`
Remove delegation | Provider | Free | [`DelegationRevoked`](https://frequency-chain.github.io/frequency/pallet_msa/pallet/enum.Event.html#variant.DelegationRevoked) | 1 | +| `withdraw_tokens`
Withdraw all tokens from an MSA | Token Account | Tokens | [`Transfer`](https://paritytech.github.io/polkadot-sdk/master/pallet_balances/pallet/enum.Event.html#variant.Transfer) | 158 | See [Rust Docs](https://frequency-chain.github.io/frequency/pallet_msa/pallet/struct.Pallet.html) for more details. diff --git a/pallets/msa/src/benchmarking.rs b/pallets/msa/src/benchmarking.rs index 2ccc3e05cd..f2aa6ff38d 100644 --- a/pallets/msa/src/benchmarking.rs +++ b/pallets/msa/src/benchmarking.rs @@ -5,7 +5,7 @@ use crate::types::EMPTY_FUNCTION; #[allow(unused)] use crate::Pallet as Msa; use frame_benchmarking::{account, v2::*}; -use frame_support::assert_ok; +use frame_support::{assert_ok, traits::fungible::Inspect}; use frame_system::RawOrigin; use sp_core::{crypto::KeyTypeId, Encode}; use sp_runtime::RuntimeAppPublic; @@ -56,6 +56,25 @@ fn add_key_payload_and_signature( (add_key_payload, MultiSignature::Sr25519(signature.into()), acc) } +fn withdraw_tokens_payload_and_signature( + msa_id: u64, + msa_key_pair: SignerId, +) -> (AuthorizedKeyData, MultiSignature, T::AccountId) { + let new_keys = SignerId::generate_pair(None); + let public_key = T::AccountId::decode(&mut &new_keys.encode()[..]).unwrap(); + let withdraw_tokens_payload = AuthorizedKeyData:: { + msa_id, + expiration: 10u32.into(), + authorized_public_key: public_key, + }; + + let encoded_withdraw_tokens_payload = wrap_binary_data(withdraw_tokens_payload.encode()); + + let signature = msa_key_pair.sign(&encoded_withdraw_tokens_payload).unwrap(); + let acc = T::AccountId::decode(&mut &new_keys.encode()[..]).unwrap(); + (withdraw_tokens_payload, MultiSignature::Sr25519(signature.into()), acc) +} + fn create_msa_account_and_keys() -> (T::AccountId, SignerId, MessageSourceId) { let key_pair = SignerId::generate_pair(None); let account_id = T::AccountId::decode(&mut &key_pair.encode()[..]).unwrap(); @@ -369,6 +388,37 @@ mod benchmarks { Ok(()) } + #[benchmark] + fn withdraw_tokens() -> Result<(), BenchmarkError> { + prep_signature_registry::(); + + let (msa_public_key, msa_key_pair, msa_id) = create_msa_account_and_keys::(); + + let eth_account_id: H160 = Msa::::msa_id_to_eth_address(msa_id); + let mut bytes = &EthereumAddressMapper::to_bytes32(ð_account_id.0)[..]; + let msa_account_id = ::AccountId::decode(&mut bytes).unwrap(); + + // Fund MSA + // let balance = <::Currency as Inspect<::AccountId>>::Balance.from(10_000_000u128); + let balance = ::Currency::minimum_balance(); + T::Currency::set_balance(&msa_account_id, balance); + assert_eq!(T::Currency::balance(&msa_account_id), balance); + + let (add_key_payload, owner_signature, new_account_id) = + withdraw_tokens_payload_and_signature::(msa_id, msa_key_pair); + + #[extrinsic_call] + _( + RawOrigin::Signed(new_account_id.clone()), + msa_public_key.clone(), + owner_signature, + add_key_payload, + ); + + assert_eq!(T::Currency::balance(&msa_account_id), Zero::zero()); + Ok(()) + } + impl_benchmark_test_suite!( Msa, crate::tests::mock::new_test_ext_keystore(), diff --git a/pallets/msa/src/lib.rs b/pallets/msa/src/lib.rs index a19359ed7e..d9b7d22a6e 100644 --- a/pallets/msa/src/lib.rs +++ b/pallets/msa/src/lib.rs @@ -34,7 +34,13 @@ use frame_support::{ dispatch::{DispatchInfo, DispatchResult, PostDispatchInfo}, pallet_prelude::*, - traits::IsSubType, + traits::{ + tokens::{ + fungible::{Inspect as InspectFungible, Mutate}, + Fortitude, Preservation, + }, + IsSubType, + }, }; use lazy_static::lazy_static; use parity_scale_codec::{Decode, Encode}; @@ -53,6 +59,7 @@ use common_primitives::{ }, node::ProposalProvider, schema::{SchemaId, SchemaValidator}, + signatures::{AccountAddressMapper, EthereumAddressMapper}, }; use frame_system::pallet_prelude::*; use scale_info::TypeInfo; @@ -75,7 +82,9 @@ pub use common_primitives::{ handles::HandleProvider, msa::MessageSourceId, node::EIP712Encode, utils::wrap_binary_data, }; pub use pallet::*; -pub use types::{AddKeyData, AddProvider, PermittedDelegationSchemas, EMPTY_FUNCTION}; +pub use types::{ + AddKeyData, AddProvider, AuthorizedKeyData, PermittedDelegationSchemas, EMPTY_FUNCTION, +}; pub use weights::*; /// Offchain storage for MSA pallet @@ -143,6 +152,9 @@ pub mod pallet { /// The Council proposal provider interface type ProposalProvider: ProposalProvider; + + /// Currency type for managing MSA account balances + type Currency: Mutate + InspectFungible; } const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); @@ -306,7 +318,7 @@ pub mod pallet { /// MsaId values have reached the maximum MsaIdOverflow, - /// Cryptographic signature verification failed for adding a key to MSA + /// Cryptographic signature verification failed MsaOwnershipInvalidSignature, /// Ony the MSA Owner may perform the operation @@ -386,6 +398,12 @@ pub mod pallet { /// Attempted to add a new signature to a corrupt signature registry SignatureRegistryCorrupted, + + /// Account has no/insufficient balance to withdraw + InsufficientBalanceToWithdraw, + + /// Fund transfer error + UnexpectedTokenTransferError, } impl BlockNumberProvider for Pallet { @@ -905,6 +923,67 @@ pub mod pallet { Ok(()) } + + /// Withdraw all available tokens from the account associated with the MSA, to the account of the caller. + /// + /// The `origin` parameter represents the account from which the function is called and must be the account receiving the tokens. + /// + /// The function requires one signature: `msa_owner_proof`, which serve as proof that the owner of the MSA authorizes the transaction. + /// + /// The necessary information for the withdrawal authorization, the destination account and the MSA ID, are contained in the `authorization_payload` parameter of type [AddKeyData]. + /// It also contains an expiration block number for the proof, ensuring it is valid and must be greater than the current block. + /// + /// # Events + /// * [`pallet_balances::Event::::Transfer`](https://docs.rs/pallet-balances/latest/pallet_balances/pallet/enum.Event.html#variant.Transfer) - Transfer token event + /// + /// # Errors + /// + /// * [`Error::ProofHasExpired`] - the current block is less than the `expired` block number set in `AddKeyData`. + /// * [`Error::ProofNotYetValid`] - the `expired` block number set in `AddKeyData` is greater than the current block number plus mortality_block_limit(). + /// * [`Error::SignatureAlreadySubmitted`] - signature has already been used. + /// * [`Error::InsufficientBalanceToWithdraw`] - the MSA account has not balance to withdraw + /// * [`Error::UnexpectedTokenTransferError`] - the token transfer failed + /// + #[pallet::call_index(14)] + #[pallet::weight((T::WeightInfo::withdraw_tokens(), DispatchClass::Normal, Pays::No))] + pub fn withdraw_tokens( + origin: OriginFor, + _msa_owner_public_key: T::AccountId, + msa_owner_proof: MultiSignature, + authorization_payload: AuthorizedKeyData, + ) -> DispatchResult { + let public_key = ensure_signed(origin)?; + + Self::register_signature(&msa_owner_proof, authorization_payload.expiration)?; + + let msa_id = authorization_payload.msa_id; + + // - Get account address for MSA + let msa_address = Self::msa_id_to_eth_address(msa_id); + + // - Convert to AccountId + let mut bytes = &EthereumAddressMapper::to_bytes32(&msa_address.0)[..]; + let msa_account_id = T::AccountId::decode(&mut bytes).unwrap(); + + // Get balance to transfer + let msa_balance = T::Currency::reducible_balance( + &msa_account_id, + Preservation::Expendable, + Fortitude::Polite, + ); + ensure!(msa_balance > Zero::zero(), Error::::InsufficientBalanceToWithdraw); + + // Transfer balance to the caller + let result = ::Currency::transfer( + &msa_account_id, + &public_key, + msa_balance, + Preservation::Expendable, + ); + ensure!(result.is_ok(), Error::::UnexpectedTokenTransferError); + + Ok(()) + } } } @@ -1425,12 +1504,6 @@ impl Pallet { /// Raises `SignatureAlreadySubmitted` if the signature exists in the registry. /// Raises `SignatureRegistryLimitExceeded` if the oldest signature of the list has not yet expired. /// - /// Example list: - /// - `1,2 (oldest)` - /// - `2,3` - /// - `3,4` - /// - 4 (newest in pointer storage)` - /// /// # Errors /// * [`Error::ProofNotYetValid`] /// * [`Error::ProofHasExpired`] @@ -1441,7 +1514,24 @@ impl Pallet { signature: &MultiSignature, signature_expires_at: BlockNumberFor, ) -> DispatchResult { - let current_block = frame_system::Pallet::::block_number(); + let current_block = Self::check_signature(signature, signature_expires_at)?; + + Self::enqueue_signature(signature, signature_expires_at, current_block) + } + + /// Check that mortality_block is within bounds. + /// Raises `SignatureAlreadySubmitted` if the signature exists in the registry. + /// + /// # Errors + /// * [`Error::ProofNotYetValid`] + /// * [`Error::ProofHasExpired`] + /// * [`Error::SignatureAlreadySubmitted`] + /// + pub fn check_signature( + signature: &MultiSignature, + signature_expires_at: BlockNumberFor, + ) -> Result, DispatchError> { + let current_block: BlockNumberFor = frame_system::Pallet::::block_number(); let max_lifetime = Self::mortality_block_limit(current_block); ensure!(max_lifetime > signature_expires_at, Error::::ProofNotYetValid); @@ -1452,11 +1542,35 @@ impl Pallet { !>::contains_key(signature), Error::::SignatureAlreadySubmitted ); + if let Some(signature_pointer) = PayloadSignatureRegistryPointer::::get() { + ensure!(signature_pointer.newest != *signature, Error::::SignatureAlreadySubmitted); + } - Self::enqueue_signature(signature, signature_expires_at, current_block) + Ok(current_block) } /// Do the actual enqueuing into the list storage and update the pointer + /// + /// The signature registry consist of two storage items: + /// - `PayloadSignatureRegistryList` - a linked list of signatures, in which each entry + /// points to the next newest signature. The list is stored as a mapping of + /// `MultiSignature` to a tuple of `(BlockNumberFor, MultiSignature)`. + /// The tuple contains the expiration block number and the next signature in the list. + /// - `PayloadSignatureRegistryPointer` - a struct containing the newest signature, + /// the oldest signature, the count of signatures, and the expiration block number of the newest signature. + /// + /// NOTE: 'newest' and 'oldest' refer to the order in which the signatures were added to the list, + /// which is not necessarily the order in which they expire. + /// + /// Example: (key [signature], value [next newest signature]) + /// - `1,2 (oldest)` + /// - `2,3` + /// - `3,4` + /// - 4 (newest in pointer storage)` + /// + // DEVELOPER NOTE: As currently implemented, the signature registry list will continue to grow until it reaches + // the maximum number of signatures, at which point it will remain at that size, only ever replacing the oldest + // signature with the newest one. This is a trade-off between storage space and performance. fn enqueue_signature( signature: &MultiSignature, signature_expires_at: BlockNumberFor, @@ -1828,6 +1942,84 @@ impl CheckFreeExtrinsicUse { ValidTransaction::with_tag_prefix(TAG_PREFIX).and_provides(account_id).build() } + + /// Validates that a request to withdraw tokens from an MSA passes the following checks: + /// - Receiver (origin) is NOT a control key for any MSA + /// - Signed payload matches MSA ID and signature validation + /// - Signature is not a duplicate + /// - MSA has tokens available to be withdrawn + /// - Receiver account has/will have enough balance to avoid being reaped + /// + /// # Errors + /// + /// * [`Error::NotKeyOwner`] - the transaction signer origin is not the owner of the public key contained in the provided `authorization_payload` + /// * [`Error::MsaOwnershipInvalidSignature`] - `msa_owner_public_key` is not a valid signer of the provided `authorization_payload`. + /// * [`Error::NoKeyExists`] - the public key supplied in `msa_owner_public_key` is not a registered control key of any MSA. + /// * [`Error::NotMsaOwner`] - the MSA ID in `authorization_payload` does not match the MSA ID of the `msa_owner_public_key`. + /// * [`Error::ProofHasExpired`] - the current block is less than the `expired` block number set in `AddKeyData`. + /// * [`Error::ProofNotYetValid`] - the `expired` block number set in `AddKeyData` is greater than the current block number plus mortality_block_limit(). + /// * [`Error::SignatureAlreadySubmitted`] - signature has already been used. + /// * [`Error::InsufficientBalanceToWithdraw`] - the MSA account has not balance to withdraw + /// * [`Error::InsufficientBalanceToFundDestination`] - the transfer would result in an account that would be reaped due to existential balance requirements + /// + pub fn validate_msa_token_withdrawal( + receiver_account_id: &T::AccountId, + msa_owner_public_key: &T::AccountId, + msa_owner_proof: &MultiSignature, + authorization_payload: &AuthorizedKeyData, + ) -> TransactionValidity { + const TAG_PREFIX: &str = "MsaTokenWithdrawal"; + + ensure!( + *receiver_account_id == authorization_payload.authorized_public_key, + InvalidTransaction::Custom(ValidityError::NotKeyOwner as u8) + ); + + ensure!( + Pallet::::verify_signature( + msa_owner_proof, + msa_owner_public_key, + authorization_payload + ), + InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8) + ); + + // Origin must NOT be an MSA control key + ensure!( + !PublicKeyToMsaId::::contains_key(receiver_account_id), + InvalidTransaction::Custom(ValidityError::IneligibleOrigin as u8) + ); + + Pallet::::check_signature(msa_owner_proof, authorization_payload.expiration).map_err( + |_| InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8), + )?; + + let msa_id = authorization_payload.msa_id; + + Pallet::::ensure_msa_owner(msa_owner_public_key, msa_id) + .map_err(|_| InvalidTransaction::Custom(ValidityError::InvalidMsaKey as u8))?; + + // - Get account address for MSA + let msa_address = Pallet::::msa_id_to_eth_address(msa_id); + + // - Convert to AccountId + let mut bytes = &EthereumAddressMapper::to_bytes32(&msa_address.0)[..]; + let msa_account_id = T::AccountId::decode(&mut bytes).unwrap(); + + // - Check that the MSA has a balance to withdraw + let msa_balance = T::Currency::reducible_balance( + &msa_account_id, + Preservation::Expendable, + Fortitude::Polite, + ); + ensure!( + msa_balance > Zero::zero(), + InvalidTransaction::Custom(ValidityError::InsufficientBalanceToWithdraw as u8) + ); + ValidTransaction::with_tag_prefix(TAG_PREFIX) + .and_provides(receiver_account_id) + .build() + } } /// Errors related to the validity of the CheckFreeExtrinsicUse signed extension. @@ -1848,6 +2040,12 @@ pub enum ValidityError { InvalidNonZeroProviderDelegations, /// HandleNotRetired HandleNotRetired, + /// Cryptographic signature verification failed + MsaOwnershipInvalidSignature, + /// MSA balance is zero + InsufficientBalanceToWithdraw, + /// Origin is ineleligible for the current transaction + IneligibleOrigin, } impl CheckFreeExtrinsicUse { @@ -1900,6 +2098,7 @@ where /// * revoke_delegation_by_delegator /// * delete_msa_public_key /// * retire_msa + /// * withdraw_tokens /// /// Validate functions for the above MUST prevent errors in the extrinsic logic to prevent spam. /// @@ -1924,6 +2123,16 @@ where Some(Call::delete_msa_public_key { public_key_to_delete, .. }) => CheckFreeExtrinsicUse::::validate_key_delete(who, public_key_to_delete), Some(Call::retire_msa { .. }) => CheckFreeExtrinsicUse::::ensure_msa_can_retire(who), + Some(Call::withdraw_tokens { + msa_owner_public_key, + msa_owner_proof, + authorization_payload, + }) => CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + who, + msa_owner_public_key, + msa_owner_proof, + authorization_payload, + ), _ => Ok(Default::default()), } } diff --git a/pallets/msa/src/tests/mock.rs b/pallets/msa/src/tests/mock.rs index 87f167abc6..74e67e7f27 100644 --- a/pallets/msa/src/tests/mock.rs +++ b/pallets/msa/src/tests/mock.rs @@ -5,7 +5,7 @@ use common_primitives::{ use common_runtime::constants::DAYS; use frame_support::{ assert_ok, parameter_types, - traits::{ConstU16, ConstU32, EitherOfDiverse, OnFinalize, OnInitialize}, + traits::{ConstU16, ConstU32, ConstU64, EitherOfDiverse, OnFinalize, OnInitialize}, weights::Weight, }; use frame_system::EnsureRoot; @@ -38,6 +38,7 @@ frame_support::construct_runtime!( pub enum Test { System: frame_system::{Pallet, Call, Config, Storage, Event}, + Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Msa: pallet_msa::{Pallet, Call, Storage, Event}, Schemas: pallet_schemas::{Pallet, Call, Storage, Event}, Council: pallet_collective::::{Pallet, Call, Config, Storage, Event, Origin}, @@ -97,7 +98,7 @@ impl frame_system::Config for Test { type BlockHashCount = ConstU32<250>; type Version = (); type PalletInfo = PalletInfo; - type AccountData = (); + type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); type SystemWeightInfo = (); @@ -112,6 +113,23 @@ impl frame_system::Config for Test { type ExtensionsWeightInfo = (); } +impl pallet_balances::Config for Test { + type MaxReserves = (); + type ReserveIdentifier = [u8; 8]; + type MaxLocks = ConstU32<10>; + type Balance = u64; + type RuntimeEvent = RuntimeEvent; + type DustRemoval = (); + type ExistentialDeposit = ConstU64<1>; + type AccountStore = System; + type WeightInfo = (); + type FreezeIdentifier = RuntimeFreezeReason; + type MaxFreezes = ConstU32<2>; + type RuntimeHoldReason = (); + type RuntimeFreezeReason = (); + type DoneSlashHandler = (); +} + impl pallet_schemas::Config for Test { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); @@ -215,6 +233,7 @@ impl pallet_msa::Config for Test { EnsureRoot, pallet_collective::EnsureMembers, >; + type Currency = pallet_balances::Pallet; } pub fn set_max_signature_stored(max: u32) { diff --git a/pallets/msa/src/tests/mod.rs b/pallets/msa/src/tests/mod.rs index 786cd0482d..5d3d76b092 100644 --- a/pallets/msa/src/tests/mod.rs +++ b/pallets/msa/src/tests/mod.rs @@ -3,6 +3,7 @@ pub mod mock; mod creation_tests; mod delegation_tests; mod governance_tests; +mod msa_token_tests; mod offchain_tests; mod other_tests; mod public_key_tests; diff --git a/pallets/msa/src/tests/msa_token_tests.rs b/pallets/msa/src/tests/msa_token_tests.rs new file mode 100644 index 0000000000..5145d4e196 --- /dev/null +++ b/pallets/msa/src/tests/msa_token_tests.rs @@ -0,0 +1,313 @@ +use frame_support::{ + assert_err, assert_noop, assert_ok, + pallet_prelude::InvalidTransaction, + traits::{ + tokens::{fungible::Inspect, Fortitude, Preservation}, + Currency, + }, +}; + +use sp_core::{sr25519, Encode, Pair}; +use sp_runtime::MultiSignature; + +use crate::{ + tests::mock::*, types::AuthorizedKeyData, CheckFreeExtrinsicUse, Config, Error, ValidityError, +}; + +use common_primitives::{ + msa::{MessageSourceId, H160}, + node::BlockNumber, + signatures::{AccountAddressMapper, EthereumAddressMapper}, + utils::wrap_binary_data, +}; + +use pallet_balances::Event as BalancesEvent; + +fn generate_payload( + msa_id: MessageSourceId, + msa_owner_keys: &sr25519::Pair, + authorized_public_key: &sr25519::Pair, + expiration: Option, +) -> (AuthorizedKeyData, Vec, MultiSignature) { + let payload = AuthorizedKeyData:: { + msa_id, + expiration: match expiration { + Some(block_number) => block_number, + None => 10, + }, + authorized_public_key: authorized_public_key.public().into(), + }; + + let encoded_payload = wrap_binary_data(payload.encode()); + let signature: MultiSignature = msa_owner_keys.sign(&encoded_payload).into(); + + (payload, encoded_payload, signature) +} + +#[test] +fn it_fails_when_caller_key_does_not_match_payload() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + let (other_key_pair, _) = sr25519::Pair::generate(); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &other_key_pair, None); + + assert_noop!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::NotKeyOwner as u8) + ); + }); +} + +#[test] +fn it_fails_when_payload_signature_is_invalid() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + let (other_key_pair, _) = sr25519::Pair::generate(); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &other_key_pair, &origin_key_pair, None); + + assert_noop!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8) + ); + }); +} + +#[test] +fn it_fails_when_proof_is_expired() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + + // The current block is 1, therefore setting the proof expiration to 1 should cause + // the extrinsic to fail because the proof has expired. + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &origin_key_pair, Some(1)); + + assert_noop!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8) + ); + }); +} + +#[test] +fn it_fails_when_proof_is_not_yet_valid() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + + // The current block is 1, therefore setting the proof expiration to the max mortality period + // should cause the extrinsic to fail + let (payload, _, msa_signature) = generate_payload( + msa_id, + &owner_key_pair, + &origin_key_pair, + Some(Msa::mortality_block_limit(1)), + ); + + assert_noop!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8) + ); + }); +} + +#[test] +fn it_fails_when_origin_is_an_msa_control_key() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (_, origin_key_pair) = create_account(); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &origin_key_pair, None); + + assert_noop!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::IneligibleOrigin as u8) + ); + }); +} + +#[test] +fn it_fails_when_msa_key_is_not_an_msa_control_key() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + + let (payload, _, msa_signature) = + generate_payload(msa_id + 1, &owner_key_pair, &origin_key_pair, None); + + assert_err!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::InvalidMsaKey as u8) + ); + }) +} + +#[test] +fn it_fails_when_msa_key_does_not_control_msa_in_payload() { + new_test_ext().execute_with(|| { + let (msa_id, _) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + let (other_key_pair, _) = sr25519::Pair::generate(); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &other_key_pair, &origin_key_pair, None); + + assert_err!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &other_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::InvalidMsaKey as u8) + ); + }) +} + +#[test] +fn it_fails_when_msa_does_not_have_a_balance() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &origin_key_pair, None); + + assert_err!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::InsufficientBalanceToWithdraw as u8) + ); + }) +} + +#[test] +fn it_succeeds_when_balance_is_sufficient() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + let eth_account_id: H160 = Msa::msa_id_to_eth_address(msa_id); + let bytes: [u8; 32] = EthereumAddressMapper::to_bytes32(ð_account_id.0); + let msa_account_id = ::AccountId::from(bytes); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &origin_key_pair, None); + + let transfer_amount = 10_000_000; + + // Fund MSA + let _ = ::Currency::deposit_creating(&msa_account_id, transfer_amount); + + assert_ok!(Msa::withdraw_tokens( + RuntimeOrigin::signed(origin_key_pair.public().into()), + owner_key_pair.public().into(), + msa_signature, + payload + )); + + let receiver_balance = ::Currency::reducible_balance( + &origin_key_pair.public().into(), + Preservation::Expendable, + Fortitude::Polite, + ); + assert_eq!( + receiver_balance, transfer_amount, + "transfer amount {} does not equal new balance {}", + transfer_amount, receiver_balance + ); + + System::assert_last_event(RuntimeEvent::Balances(BalancesEvent::Transfer { + from: msa_account_id, + to: origin_key_pair.public().into(), + amount: transfer_amount, + })); + }) +} + +#[test] +fn it_fails_when_duplicate_signature_submitted() { + new_test_ext().execute_with(|| { + let (msa_id, owner_key_pair) = create_account(); + let (origin_key_pair, _) = sr25519::Pair::generate(); + let eth_account_id: H160 = Msa::msa_id_to_eth_address(msa_id); + let bytes: [u8; 32] = EthereumAddressMapper::to_bytes32(ð_account_id.0); + let msa_account_id = ::AccountId::from(bytes); + + let (payload, _, msa_signature) = + generate_payload(msa_id, &owner_key_pair, &origin_key_pair, None); + + let transfer_amount = 10_000_000; + + // Fund MSA + let _ = ::Currency::deposit_creating(&msa_account_id, transfer_amount); + + assert_ok!(Msa::withdraw_tokens( + RuntimeOrigin::signed(origin_key_pair.public().into()), + owner_key_pair.public().into(), + msa_signature.clone(), + payload.clone() + )); + + assert_err!( + CheckFreeExtrinsicUse::::validate_msa_token_withdrawal( + &origin_key_pair.public().into(), + &owner_key_pair.public().into(), + &msa_signature, + &payload + ), + InvalidTransaction::Custom(ValidityError::MsaOwnershipInvalidSignature as u8) + ); + + assert_err!( + Msa::withdraw_tokens( + RuntimeOrigin::signed(origin_key_pair.public().into()), + owner_key_pair.public().into(), + msa_signature.clone(), + payload.clone() + ), + Error::::SignatureAlreadySubmitted + ); + }) +} diff --git a/pallets/msa/src/tests/other_tests.rs b/pallets/msa/src/tests/other_tests.rs index a931da3011..b5fa27f8a4 100644 --- a/pallets/msa/src/tests/other_tests.rs +++ b/pallets/msa/src/tests/other_tests.rs @@ -13,8 +13,8 @@ use crate::{ ensure, tests::mock::*, types::{AddProvider, PermittedDelegationSchemas, EMPTY_FUNCTION}, - AddKeyData, Config, DelegatorAndProviderToDelegation, DispatchResult, Error, Event, - ProviderToRegistryEntry, PublicKeyToMsaId, + AddKeyData, AuthorizedKeyData, Config, DelegatorAndProviderToDelegation, DispatchResult, Error, + Event, ProviderToRegistryEntry, PublicKeyToMsaId, }; use common_primitives::signatures::AccountAddressMapper; @@ -789,6 +789,7 @@ fn eth_address_to_checksummed_string() { } } +// Test data generated by [this tool](../../eth-migration/metamask.html) #[test] fn ethereum_eip712_signatures_for_add_key_should_work() { new_test_ext().execute_with(|| { @@ -819,6 +820,36 @@ fn ethereum_eip712_signatures_for_add_key_should_work() { }); } +#[test] +fn ethereum_eip712_signatures_for_authorized_key_should_work() { + new_test_ext().execute_with(|| { + let address = EthereumAddressMapper::to_account_id(&from_hex("0x7A23F8D62589aB9651722C7F4a0E998D7d3Ef2A9").unwrap_or_default()); + let payload: AuthorizedKeyData = AuthorizedKeyData { + msa_id: 12876327, + expiration: 100, + authorized_public_key: address.into(), + }; + let encoded_payload = payload.encode_eip_712(); + + // following signature is generated via Metamask using the same input to check compatibility + let signature_raw = from_hex("0x9dec03e5c93e2b9619cc8fd77383a8fc38d4aa3dc20fa26be436d386acb380260e2a82e677b71f28adc7cc63b60855ccc481057307a1b05dbb2f5af19c66b5461c").expect("Should convert"); + let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw( + signature_raw.try_into().expect("should convert"), + )); + + // Non-compressed public key associated with the keypair used in Metamask + // 0x509540919faacf9ab52146c9aa40db68172d83777250b28e4679176e49ccdd9fa213197dc0666e85529d6c9dda579c1295d61c417f01505765481e89a4016f02 + let public_key = ecdsa::Public::from_raw( + from_hex("0x02509540919faacf9ab52146c9aa40db68172d83777250b28e4679176e49ccdd9f") + .expect("should convert") + .try_into() + .expect("invalid size"), + ); + let unified_signer = UnifiedSigner::from(public_key); + assert!(unified_signature.verify(&encoded_payload[..], &unified_signer.into_account())); + }); +} + #[test] fn ethereum_eip712_signatures_for_add_provider_should_work() { new_test_ext().execute_with(|| { diff --git a/pallets/msa/src/types.rs b/pallets/msa/src/types.rs index 1d2bd8e568..d22604310f 100644 --- a/pallets/msa/src/types.rs +++ b/pallets/msa/src/types.rs @@ -21,7 +21,8 @@ use sp_core::U256; /// Dispatch Empty pub const EMPTY_FUNCTION: fn(MessageSourceId) -> DispatchResult = |_| Ok(()); -/// A type definition for the payload of adding an MSA key - `pallet_msa::add_public_key_to_msa` +/// A type definition for the payload for the following operation: +/// - Adding an MSA key - `pallet_msa::add_public_key_to_msa` #[derive( TypeInfo, RuntimeDebugNoBound, Clone, Decode, DecodeWithMemTracking, Encode, PartialEq, Eq, )] @@ -29,7 +30,7 @@ pub const EMPTY_FUNCTION: fn(MessageSourceId) -> DispatchResult = |_| Ok(()); pub struct AddKeyData { /// Message Source Account identifier pub msa_id: MessageSourceId, - /// The block number at which the signed proof for add_public_key_to_msa expires. + /// The block number at which a signed proof of this payload expires. pub expiration: BlockNumberFor, /// The public key to be added. pub new_public_key: T::AccountId, @@ -68,6 +69,55 @@ impl EIP712Encode for AddKeyData { } } +/// A type definition for the payload for authorizing a public key for the following operations: +/// - Authorizing a token withdrawal to an address associated with a public key - `pallet_msa::withdraw_tokens` +#[derive( + TypeInfo, RuntimeDebugNoBound, Clone, Decode, DecodeWithMemTracking, Encode, PartialEq, Eq, +)] +#[scale_info(skip_type_params(T))] +pub struct AuthorizedKeyData { + /// Message Source Account identifier + pub msa_id: MessageSourceId, + /// The block number at which a signed proof of this payload expires. + pub expiration: BlockNumberFor, + /// The public key to be added that is authorized as the target of the current operation + pub authorized_public_key: T::AccountId, +} + +impl EIP712Encode for AuthorizedKeyData { + fn encode_eip_712(&self) -> Box<[u8]> { + lazy_static! { + // get prefix and domain separator + static ref PREFIX_DOMAIN_SEPARATOR: Box<[u8]> = + get_eip712_encoding_prefix("0xcccccccccccccccccccccccccccccccccccccccc"); + + // signed payload + static ref MAIN_TYPE_HASH: [u8; 32] = sp_io::hashing::keccak_256( + b"AuthorizedKeyData(uint64 msaId,uint32 expiration,address authorizedPublicKey)", + ); + } + let coded_owner_msa_id = to_abi_compatible_number(self.msa_id); + let expiration: U256 = self.expiration.into(); + let coded_expiration = to_abi_compatible_number(expiration.as_u128()); + let converted_public_key = + T::ConvertIntoAccountId32::convert(self.authorized_public_key.clone()); + let mut zero_prefixed_address = [0u8; 32]; + zero_prefixed_address[12..] + .copy_from_slice(&EthereumAddressMapper::to_ethereum_address(converted_public_key).0); + let message = sp_io::hashing::keccak_256( + &[ + MAIN_TYPE_HASH.as_slice(), + &coded_owner_msa_id, + &coded_expiration, + &zero_prefixed_address, + ] + .concat(), + ); + let combined = [PREFIX_DOMAIN_SEPARATOR.as_ref(), &message].concat(); + combined.into_boxed_slice() + } +} + /// Structure that is signed for granting permissions to a Provider #[derive(TypeInfo, Clone, Debug, Decode, DecodeWithMemTracking, Encode, PartialEq, Eq)] pub struct AddProvider { diff --git a/pallets/msa/src/weights.rs b/pallets/msa/src/weights.rs index db27906009..f256abc2b4 100644 --- a/pallets/msa/src/weights.rs +++ b/pallets/msa/src/weights.rs @@ -45,6 +45,7 @@ pub trait WeightInfo { fn create_provider_via_governance() -> Weight; fn propose_to_be_provider() -> Weight; fn reindex_offchain() -> Weight; + fn withdraw_tokens() -> Weight; } /// Weights for `pallet_msa` using the Substrate node and recommended hardware. @@ -238,6 +239,13 @@ impl WeightInfo for SubstrateWeight { // Minimum execution time: 4_000_000 picoseconds. Weight::from_parts(8_000_000, 0) } + fn withdraw_tokens() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(8_000_000, 0) + } } // For backwards compatibility and tests. @@ -430,6 +438,23 @@ impl WeightInfo for () { // Minimum execution time: 4_000_000 picoseconds. Weight::from_parts(8_000_000, 0) } + /// Storage: `Msa::PayloadSignatureRegistryList` (r:2 w:2) + /// Proof: `Msa::PayloadSignatureRegistryList` (`max_values`: Some(50000), `max_size`: Some(144), added: 2124, mode: `MaxEncodedLen`) + /// Storage: `Msa::PayloadSignatureRegistryPointer` (r:1 w:1) + /// Proof: `Msa::PayloadSignatureRegistryPointer` (`max_values`: Some(1), `max_size`: Some(140), added: 635, mode: `MaxEncodedLen`) + /// Storage: `Msa::PublicKeyToMsaId` (r:1 w:0) + /// Proof: `Msa::PublicKeyToMsaId` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:2 w:2) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + fn withdraw_tokens() -> Weight { + // Proof Size summary in bytes: + // Measured: `1270` + // Estimated: `6691` + // Minimum execution time: 151_000_000 picoseconds. + Weight::from_parts(162_000_000, 6691) + .saturating_add(RocksDbWeight::get().reads(6_u64)) + .saturating_add(RocksDbWeight::get().writes(5_u64)) + } } @@ -587,4 +612,16 @@ mod tests { > 4107 ); } + #[test] + fn test_withdraw_tokens() { + assert!( + BlockWeights::get() + .per_class + .get(frame_support::dispatch::DispatchClass::Normal) + .max_extrinsic + .unwrap_or_else(::max_value) + .proof_size() + > 6691 + ); + } } diff --git a/runtime/frequency/src/lib.rs b/runtime/frequency/src/lib.rs index 3fc4dc6d3e..6fc2a67b03 100644 --- a/runtime/frequency/src/lib.rs +++ b/runtime/frequency/src/lib.rs @@ -435,7 +435,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: Cow::Borrowed("frequency"), impl_name: Cow::Borrowed("frequency"), authoring_version: 1, - spec_version: 158, + spec_version: 159, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -449,7 +449,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: Cow::Borrowed("frequency-testnet"), impl_name: Cow::Borrowed("frequency"), authoring_version: 1, - spec_version: 158, + spec_version: 159, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -587,6 +587,8 @@ impl pallet_msa::Config for Runtime { EnsureRoot, pallet_collective::EnsureMembers, >; + // The Currency type for managing MSA token balances + type Currency = Balances; } parameter_types! { diff --git a/tools/eth-migration/metamask.html b/tools/eth-migration/metamask.html index fe4e147468..25140b079a 100644 --- a/tools/eth-migration/metamask.html +++ b/tools/eth-migration/metamask.html @@ -331,6 +331,51 @@ alert("Signature: " + signature) }); + document.getElementById('authorized_key_payload')?.addEventListener("click", async function () { + const signature_request = { + "types": { + ...eip712domain_type_definition, + "AuthorizedKeyData": [ + { + "name": "msaId", + "type": "uint64" + }, + { + "name": "expiration", + "type": "uint32" + }, + { + "name": "authorizedPublicKey", + "type": "address" + }, + ], + }, + "primaryType": "AuthorizedKeyData", + "domain": karma_request_domain, + "message": { + "msaId": 12876327, + "expiration": 100, + "authorizedPublicKey": '0x7A23F8D62589aB9651722C7F4a0E998D7d3Ef2A9', + } + } + console.log(JSON.stringify({ + "method": "eth_signTypedData_v4", + "params": [ + accounts[0], + signature_request + ] + },null, 2)); + let signature = await window.ethereum.request({ + "method": "eth_signTypedData_v4", + "params": [ + accounts[0], + signature_request + ] + }) + console.log(signature); + alert("Signature: " + signature) + }); + document.getElementById('add_provider_request')?.addEventListener("click", async function () { const signature_request = { "types": { @@ -391,5 +436,6 @@

+