From a70704f14701a8048d1da40a64c38fe92305ff35 Mon Sep 17 00:00:00 2001 From: bwty Date: Mon, 9 Aug 2021 18:19:25 +0800 Subject: [PATCH] test: execute only once and include txIndex in signature --- contracts/MultiSigFlowToken.cdc | 4 ++- contracts/OnChainMultiSig.cdc | 35 ++++++++++++++++++-------- lib/go/keys/keys.go | 22 ++++++++-------- lib/go/keys/keys_test.go | 15 ++++++++--- lib/go/util.go | 19 +++++++++++--- lib/go/vault/vault.go | 11 ++++---- lib/go/vault/vault_test.go | 27 ++++++++++++-------- transactions/add_new_payload.cdc | 4 +-- transactions/add_payload_signature.cdc | 2 +- 9 files changed, 91 insertions(+), 48 deletions(-) diff --git a/contracts/MultiSigFlowToken.cdc b/contracts/MultiSigFlowToken.cdc index 236cb16..5916f82 100644 --- a/contracts/MultiSigFlowToken.cdc +++ b/contracts/MultiSigFlowToken.cdc @@ -77,7 +77,9 @@ pub contract MultiSigFlowToken: FungibleToken { /// To execute the multisig transaction iff conditions are met pub fun executeTx(txIndex: UInt64): @AnyResource? { let manager = OnChainMultiSig.Manager(sigStore: self.signatureStore); - let p = manager.readyForExecution(txIndex: txIndex) ?? panic ("TX not ready for execution") + let exeDetails = manager.readyForExecution(txIndex: txIndex) ?? panic ("no transactable payload at given txIndex") + let p = exeDetails.payload + self.signatureStore = exeDetails.signatureStore switch p.method { case "configureKey": let pubKey = p.args[0] as? String ?? panic ("cannot downcast public key"); diff --git a/contracts/OnChainMultiSig.cdc b/contracts/OnChainMultiSig.cdc index 9848719..4e4486a 100644 --- a/contracts/OnChainMultiSig.cdc +++ b/contracts/OnChainMultiSig.cdc @@ -6,14 +6,25 @@ pub contract OnChainMultiSig { pub event NewPayloadSigAdded(resourceId: UInt64, txIndex: UInt64); pub struct PayloadDetails { + pub var txIndex: UInt64; pub var method: String; pub var args: [AnyStruct]; - init(method: String, args: [AnyStruct]) { + init(txIndex: UInt64, method: String, args: [AnyStruct]) { + self.txIndex = txIndex; self.method = method; self.args = args; } - + } + + pub struct ExecutionDetails { + pub var payload: OnChainMultiSig.PayloadDetails; + pub var signatureStore: OnChainMultiSig.SignatureStore; + + init(payload: OnChainMultiSig.PayloadDetails, signatureStore: OnChainMultiSig.SignatureStore) { + self.payload = payload + self.signatureStore = signatureStore + } } pub struct PubKeyAttr{ @@ -50,7 +61,7 @@ pub contract OnChainMultiSig { pub fun getSignableData(payload: PayloadDetails): [UInt8]; pub fun addNewPayload (resourceId: UInt64, payload: PayloadDetails, publicKey: String, sig: [UInt8]): SignatureStore; pub fun addPayloadSignature (resourceId: UInt64, txIndex: UInt64, publicKey: String, sig: [UInt8]): SignatureStore; - pub fun readyForExecution(txIndex: UInt64): PayloadDetails?; + pub fun readyForExecution(txIndex: UInt64): ExecutionDetails?; } pub struct SignatureStore { @@ -87,7 +98,8 @@ pub contract OnChainMultiSig { pub var signatureStore: SignatureStore; pub fun getSignableData(payload: PayloadDetails): [UInt8] { - var s = payload.method.utf8; + var s = payload.txIndex.toBigEndianBytes(); + s = s.concat(payload.method.utf8); for a in payload.args { var b: [UInt8] = []; switch a.getType() { @@ -133,6 +145,11 @@ pub contract OnChainMultiSig { pub fun addNewPayload (resourceId: UInt64, payload: PayloadDetails, publicKey: String, sig: [UInt8]): SignatureStore { assert(self.signatureStore.keyList.containsKey(publicKey), message: "Public key is not a registered signer"); + let txIndex = self.signatureStore.txIndex + UInt64(1); + assert(payload.txIndex == txIndex, message: "Incorrect txIndex provided in paylaod") + assert(!self.signatureStore.payloads.containsKey(txIndex), message: "Payload index already exist"); + self.signatureStore.txIndex = txIndex; + // The keyIndex is also 0 for the first key let keyListSig = [Crypto.KeyListSignature(keyIndex: 0, signature: sig)] @@ -142,10 +159,6 @@ pub contract OnChainMultiSig { panic ("invalid signer") } - let txIndex = self.signatureStore.txIndex.saturatingAdd(1); - self.signatureStore.txIndex = txIndex; - assert(!self.signatureStore.payloads.containsKey(txIndex), message: "Payload index already exist"); - self.signatureStore.payloads.insert(key: txIndex, payload); let payloadSigDetails = PayloadSigDetails( @@ -185,7 +198,7 @@ pub contract OnChainMultiSig { return self.signatureStore } - pub fun readyForExecution(txIndex: UInt64): PayloadDetails? { + pub fun readyForExecution(txIndex: UInt64): ExecutionDetails? { assert(self.signatureStore.payloads.containsKey(txIndex), message: "No payload for such index"); let pks = self.signatureStore.payloadSigs[txIndex]!.pubKeys; let sigs = self.signatureStore.payloadSigs[txIndex]!.keyListSignatures; @@ -196,9 +209,9 @@ pub contract OnChainMultiSig { if (approvalWeight! >= 1000.0) { self.signatureStore.payloadSigs.remove(key: txIndex)!; let pd = self.signatureStore.payloads.remove(key: txIndex)!; - return pd; + return ExecutionDetails(payload: pd, signatureStore: self.signatureStore); } else { - return nil; + return nil } } diff --git a/lib/go/keys/keys.go b/lib/go/keys/keys.go index 90c7585..e11f52c 100644 --- a/lib/go/keys/keys.go +++ b/lib/go/keys/keys.go @@ -24,10 +24,11 @@ func MultiSig_RemoveKey( txIndex uint64, signerAcct string, vaultAcct string, + newPayload bool, ) (events []*gwtf.FormatedEvent, err error) { method := "removeKey" pkToRemove := cadence.NewString(g.Accounts[acctToRemove].PrivateKey.PublicKey().String()[2:]) - signable, err := util.GetSignableDataFromScript(g, method, pkToRemove) + signable, err := util.GetSignableDataFromScript(g, txIndex, method, pkToRemove) if err != nil { return } @@ -37,11 +38,11 @@ func MultiSig_RemoveKey( return } - if txIndex != 0 { - return util.MultiSig_VaultAddPayloadSignature(g, txIndex, sig, signerAcct, vaultAcct) - } else { + if newPayload { args := []cadence.Value{pkToRemove} - return util.MultiSig_VaultNewPayload(g, sig, method, args, signerAcct, vaultAcct) + return util.MultiSig_VaultNewPayload(g, sig, txIndex, method, args, signerAcct, vaultAcct) + } else { + return util.MultiSig_VaultAddPayloadSignature(g, sig, txIndex, signerAcct, vaultAcct) } } @@ -52,6 +53,7 @@ func MultiSig_ConfigKey( txIndex uint64, signerAcct string, vaultAcct string, + newPayload bool, ) (events []*gwtf.FormatedEvent, err error) { method := "configureKey" @@ -61,7 +63,7 @@ func MultiSig_ConfigKey( if err != nil { return } - signable, err := util.GetSignableDataFromScript(g, method, pkToConfig, weightToConfig) + signable, err := util.GetSignableDataFromScript(g, txIndex, method, pkToConfig, weightToConfig) if err != nil { return } @@ -71,10 +73,10 @@ func MultiSig_ConfigKey( return } - if txIndex != 0 { - return util.MultiSig_VaultAddPayloadSignature(g, txIndex, sig, signerAcct, vaultAcct) - } else { + if newPayload { args := []cadence.Value{pkToConfig, weightToConfig} - return util.MultiSig_VaultNewPayload(g, sig, method, args, signerAcct, vaultAcct) + return util.MultiSig_VaultNewPayload(g, sig, txIndex, method, args, signerAcct, vaultAcct) + } else { + return util.MultiSig_VaultAddPayloadSignature(g, sig, txIndex, signerAcct, vaultAcct) } } diff --git a/lib/go/keys/keys_test.go b/lib/go/keys/keys_test.go index 58ef7d6..18dd877 100644 --- a/lib/go/keys/keys_test.go +++ b/lib/go/keys/keys_test.go @@ -20,7 +20,10 @@ func TestAddNewPendingKeyRemoval(t *testing.T) { assert.NoError(t, err) assert.Equal(t, hasKey, true) - _, err = MultiSig_RemoveKey(g, vault.Acct500_1, 0, vault.Acct1000, vaultAcct) + initTxIndex, err := util.GetTxIndex(g, vaultAcct) + assert.NoError(t, err) + + _, err = MultiSig_RemoveKey(g, vault.Acct500_1, initTxIndex+uint64(1), vault.Acct1000, vaultAcct, true) assert.NoError(t, err) postTxIndex, err := util.GetTxIndex(g, vaultAcct) @@ -40,7 +43,10 @@ func TestRemovedKeyCannotAddSig(t *testing.T) { vaultAcct := "vaulted-account" removedAcct := vault.Acct500_1 - _, err := MultiSig_RemoveKey(g, removedAcct, 1, removedAcct, vaultAcct) + txIndex, err := util.GetTxIndex(g, vaultAcct) + assert.NoError(t, err) + + _, err = MultiSig_RemoveKey(g, removedAcct, txIndex, removedAcct, vaultAcct, false) assert.Error(t, err) } @@ -52,7 +58,10 @@ func TestAddNewPendingKeyConfig(t *testing.T) { newAcct := vault.Acct500_1 newAcctWeight := "100.00000000" - _, err := MultiSig_ConfigKey(g, newAcct, newAcctWeight, 0, vault.Acct1000, vaultAcct) + initTxIndex, err := util.GetTxIndex(g, vaultAcct) + assert.NoError(t, err) + + _, err = MultiSig_ConfigKey(g, newAcct, newAcctWeight, initTxIndex+uint64(1), vault.Acct1000, vaultAcct, true) assert.NoError(t, err) postTxIndex, err := util.GetTxIndex(g, vaultAcct) diff --git a/lib/go/util.go b/lib/go/util.go index 066d1ab..7a5b941 100644 --- a/lib/go/util.go +++ b/lib/go/util.go @@ -209,13 +209,22 @@ func SignPayloadOffline(g *gwtf.GoWithTheFlow, message []byte, signingAcct strin func GetSignableDataFromScript( g *gwtf.GoWithTheFlow, + txIndex uint64, method string, args ...cadence.Value, ) (signable []byte, err error) { filename := "../../../scripts/calc_signable_data.cdc" script := ParseCadenceTemplate(filename) + ctxIndex, err := g.ScriptFromFile(filename, script).Argument(cadence.NewOptional(cadence.UInt64(txIndex))).RunReturns() + if err != nil { + return + } + signable = append(signable, ConvertCadenceByteArray(ctxIndex)...) cMethod, err := g.ScriptFromFile(filename, script).Argument(cadence.NewOptional(cadence.String(method))).RunReturns() + if err != nil { + return + } signable = append(signable, ConvertCadenceByteArray(cMethod)...) for _, arg := range args { @@ -232,6 +241,7 @@ func GetSignableDataFromScript( func MultiSig_VaultNewPayload( g *gwtf.GoWithTheFlow, sig string, + txIndex uint64, method string, args []cadence.Value, signerAcct string, @@ -243,11 +253,12 @@ func MultiSig_VaultNewPayload( signerPubKey := g.Accounts[signerAcct].PrivateKey.PublicKey().String() e, err := g.TransactionFromFile(txFilename, txScript). SignProposeAndPayAs(signerAcct). - AccountArgument(resourceAcct). - StringArgument(signerPubKey[2:]). StringArgument(sig). + UInt64Argument(txIndex). StringArgument(method). Argument(cadence.NewArray(args)). + StringArgument(signerPubKey[2:]). + AccountArgument(resourceAcct). Run() events = ParseTestEvents(e) return @@ -255,8 +266,8 @@ func MultiSig_VaultNewPayload( func MultiSig_VaultAddPayloadSignature( g *gwtf.GoWithTheFlow, - txIndex uint64, sig string, + txIndex uint64, signerAcct string, resourceAcct string, ) (events []*gwtf.FormatedEvent, err error) { @@ -266,9 +277,9 @@ func MultiSig_VaultAddPayloadSignature( signerPubKey := g.Accounts[signerAcct].PrivateKey.PublicKey().String() e, err := g.TransactionFromFile(txFilename, txScript). SignProposeAndPayAs(signerAcct). + StringArgument(sig). UInt64Argument(txIndex). StringArgument(signerPubKey[2:]). - StringArgument(sig). AccountArgument(resourceAcct). Run() events = ParseTestEvents(e) diff --git a/lib/go/vault/vault.go b/lib/go/vault/vault.go index 72f9162..f8cfc1e 100644 --- a/lib/go/vault/vault.go +++ b/lib/go/vault/vault.go @@ -72,6 +72,7 @@ func MultiSig_Transfer( txIndex uint64, signerAcct string, vaultAcct string, + newPaylaod bool, ) (events []*gwtf.FormatedEvent, err error) { method := "transfer" @@ -80,7 +81,7 @@ func MultiSig_Transfer( return nil, err } toAddr := cadence.BytesToAddress(g.Accounts[to].Address.Bytes()) - signable, err := util.GetSignableDataFromScript(g, method, ufix64, toAddr) + signable, err := util.GetSignableDataFromScript(g, txIndex, method, ufix64, toAddr) if err != nil { return } @@ -89,11 +90,11 @@ func MultiSig_Transfer( if err != nil { return } - if txIndex != 0 { - return util.MultiSig_VaultAddPayloadSignature(g, txIndex, sig, signerAcct, vaultAcct) - } else { + if newPaylaod { args := []cadence.Value{ufix64, toAddr} - return util.MultiSig_VaultNewPayload(g, sig, method, args, signerAcct, vaultAcct) + return util.MultiSig_VaultNewPayload(g, sig, txIndex, method, args, signerAcct, vaultAcct) + } else { + return util.MultiSig_VaultAddPayloadSignature(g, sig, txIndex, signerAcct, vaultAcct) } } diff --git a/lib/go/vault/vault_test.go b/lib/go/vault/vault_test.go index 6c642bc..5202360 100644 --- a/lib/go/vault/vault_test.go +++ b/lib/go/vault/vault_test.go @@ -41,13 +41,12 @@ func TestAddNewPendingTransferPayloadWithFullMultiSigAccount(t *testing.T) { g := gwtf.NewGoWithTheFlow("../../../flow.json") transferAmount := "15.5" transferTo := "owner" - vaultAcct := "vaulted-account" initTxIndex, err := util.GetTxIndex(g, vaultAcct) assert.NoError(t, err) - events, err := MultiSig_Transfer(g, transferAmount, transferTo, 0, Acct1000, vaultAcct) + events, err := MultiSig_Transfer(g, transferAmount, transferTo, initTxIndex+uint64(1), Acct1000, vaultAcct, true) assert.NoError(t, err) postTxIndex, err := util.GetTxIndex(g, vaultAcct) @@ -68,12 +67,13 @@ func TestAddNewPendingTransferPayloadWithFullMultiSigAccount(t *testing.T) { func TestAddNewPendingTransferPayloadUnknowAcct(t *testing.T) { g := gwtf.NewGoWithTheFlow("../../../flow.json") transferAmount := "15.5000000" + transferTo := "owner" vaultAcct := "vaulted-account" initTxIndex, err := util.GetTxIndex(g, vaultAcct) assert.NoError(t, err) - _, err = MultiSig_Transfer(g, transferAmount, "owner", 0, "non-registered-account", vaultAcct) + _, err = MultiSig_Transfer(g, transferAmount, transferTo, initTxIndex+uint64(1), "non-registered-account", vaultAcct, true) assert.Error(t, err) postTxIndex, err := util.GetTxIndex(g, vaultAcct) @@ -81,23 +81,28 @@ func TestAddNewPendingTransferPayloadUnknowAcct(t *testing.T) { assert.Equal(t, uint64(0), postTxIndex-initTxIndex) } -func TestExecutePendingTransnferFromFullAcct(t *testing.T) { +func TestExecutePendingTransnferFromFullAcctOnlyOnce(t *testing.T) { g := gwtf.NewGoWithTheFlow("../../../flow.json") transferAmount := "15.50000000" payerAcct := "owner" vaultAcct := "vaulted-account" - txIndex := uint64(1) initFromBalance, err := util.GetBalance(g, vaultAcct) assert.NoError(t, err) - _, err = MultiSig_VaultExecuteTx(g, txIndex, payerAcct, vaultAcct) + initTxIndex, err := util.GetTxIndex(g, vaultAcct) + assert.NoError(t, err) + + _, err = MultiSig_VaultExecuteTx(g, initTxIndex, payerAcct, vaultAcct) assert.NoError(t, err) postFromBalance, err := util.GetBalance(g, vaultAcct) assert.NoError(t, err) assert.Equal(t, transferAmount, (initFromBalance - postFromBalance).String()) + + _, err = MultiSig_VaultExecuteTx(g, initTxIndex, payerAcct, vaultAcct) + assert.Error(t, err) } func TestExecutePayloadWithMultipleSig(t *testing.T) { @@ -114,7 +119,7 @@ func TestExecutePayloadWithMultipleSig(t *testing.T) { initTxIndex, err := util.GetTxIndex(g, vaultAcct) assert.NoError(t, err) - _, err = MultiSig_Transfer(g, transferAmount, transferTo, 0, Acct500_1, vaultAcct) + _, err = MultiSig_Transfer(g, transferAmount, transferTo, initTxIndex+uint64(1), Acct500_1, vaultAcct, true) assert.NoError(t, err) postTxIndex, err := util.GetTxIndex(g, vaultAcct) @@ -124,7 +129,7 @@ func TestExecutePayloadWithMultipleSig(t *testing.T) { // // Add another signature; total weight now is 500 + 250 // - events, err := MultiSig_Transfer(g, transferAmount, transferTo, postTxIndex, Acct250_1, vaultAcct) + events, err := MultiSig_Transfer(g, transferAmount, transferTo, postTxIndex, Acct250_1, vaultAcct, false) assert.NoError(t, err) uuid, err := util.GetVaultUUID(g, vaultAcct) @@ -141,16 +146,16 @@ func TestExecutePayloadWithMultipleSig(t *testing.T) { // // Add another signature; total weight now is 500 + 250 + 500 // - _, err = MultiSig_Transfer(g, transferAmount, transferTo, postTxIndex, Acct500_2, vaultAcct) + _, err = MultiSig_Transfer(g, transferAmount, transferTo, postTxIndex, Acct500_2, vaultAcct, false) assert.NoError(t, err) - initFromBalance, err := util.GetBalance(g, "vaulted-account") + initFromBalance, err := util.GetBalance(g, vaultAcct) assert.NoError(t, err) _, err = MultiSig_VaultExecuteTx(g, postTxIndex, payerAcct, vaultAcct) assert.NoError(t, err) - postFromBalance, err := util.GetBalance(g, "vaulted-account") + postFromBalance, err := util.GetBalance(g, vaultAcct) assert.NoError(t, err) assert.Equal(t, transferAmount, (initFromBalance - postFromBalance).String()) } diff --git a/transactions/add_new_payload.cdc b/transactions/add_new_payload.cdc index f21fb09..1ce359a 100644 --- a/transactions/add_new_payload.cdc +++ b/transactions/add_new_payload.cdc @@ -3,7 +3,7 @@ import MultiSigFlowToken from 0x{{.MultiSigFlowToken}} import OnChainMultiSig from 0x{{.OnChainMultiSig}} -transaction (addr: Address, publicKey: String, sig: String, method: String, args: [AnyStruct]) { +transaction (sig: String, txIndex: UInt64, method: String, args: [AnyStruct], publicKey: String, addr: Address ) { prepare(oneOfMultiSig: AuthAccount) { } @@ -14,7 +14,7 @@ transaction (addr: Address, publicKey: String, sig: String, method: String, args .borrow<&MultiSigFlowToken.Vault{OnChainMultiSig.PublicSigner}>() ?? panic("Could not borrow vault pub sig reference") - let p = OnChainMultiSig.PayloadDetails(method: method, args: args); + let p = OnChainMultiSig.PayloadDetails(txIndex: txIndex, method: method, args: args); return pubSigRef.addNewPayload(payload: p, publicKey: publicKey, sig: sig.decodeHex()) } } diff --git a/transactions/add_payload_signature.cdc b/transactions/add_payload_signature.cdc index 0e17a85..b3dc80b 100644 --- a/transactions/add_payload_signature.cdc +++ b/transactions/add_payload_signature.cdc @@ -3,7 +3,7 @@ import MultiSigFlowToken from 0x{{.MultiSigFlowToken}} import OnChainMultiSig from 0x{{.OnChainMultiSig}} -transaction (txIndex: UInt64, publicKey: String, sig: String, addr: Address) { +transaction (sig: String, txIndex: UInt64, publicKey: String, addr: Address) { prepare(oneOfMultiSig: AuthAccount) { }