From 4e57122681c341e3350f152cf4d03c0041a5bbea Mon Sep 17 00:00:00 2001 From: Greg Nazario Date: Thu, 14 Nov 2024 15:33:55 -0500 Subject: [PATCH] [multikey] Allow variable length signatures in multikey --- CHANGELOG.md | 8 +++++++ client_test.go | 4 ++++ crypto/multiKey.go | 46 +++++++++++++++++++++++++++++++---------- crypto/multiKey_test.go | 13 ++++++++++++ signer_test.go | 24 ++++++++++++++------- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f86969..a811ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,18 +4,23 @@ All notable changes to the Aptos Go SDK will be captured in this file. This chan adheres to the format set out by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). # Unreleased + +- [`Fix`][`Breaking`] Fix MultiKey implementation to be more consistent with the rest of the SDKs - Add BCS support for optional values # v1.1.0 (11/07/2024) + - Add icon uri and project uri to FA client, reduce duplicated code - Add better error messages around script argument parsing - Add example for scripts with FA # v1.0.0 (10/28/2024) + - [`Fix`] Paginate transactions properly if no given start version - Add account transactions API # v0.7.0 (8/19/2024) + - [`Fix`] Parse GenesisTransaction properly - [`Fix`] Ensure if no block transactions are requested, it doesn't fail to fetch a block - [`Doc`] Fix comment from milliseconds to microseconds @@ -24,6 +29,7 @@ adheres to the format set out by [Keep a Changelog](https://keepachangelog.com/e - [`Fix`] Fix MultiKey signature verification and building to work with any keys # v0.6.0 (6/28/2024) + - [`Breaking`] Change type from Transaction to CommittedTransaction for cases that it's known they're committed - [`Fix`] Fix secp256k1 signing and verification to be correctly used - [`Fix`] Fix supply view function for FungibleAssetClient @@ -42,11 +48,13 @@ adheres to the format set out by [Keep a Changelog](https://keepachangelog.com/e - Change signers to have simulation authenticators by default # v0.5.0 (6/21/2024) + - [`Fix`] Fix sponsored transactions, and add example for sponsored transactions - Add examples to CI - Add node health check API to client # v0.4.1 (6/18/2024) + - [`Fix`] Make examples more portable / not rely on internal packages # v0.4.0 (6/17/2024) diff --git a/client_test.go b/client_test.go index 4325c86..89e10e9 100644 --- a/client_test.go +++ b/client_test.go @@ -47,6 +47,10 @@ func initSigners() { signer, err := NewMultiKeyTestSigner(3, 2) return any(signer).(TransactionSigner), err } + TestSigners["5-of-32 MultiKey"] = func() (TransactionSigner, error) { + signer, err := NewMultiKeyTestSigner(32, 5) + return any(signer).(TransactionSigner), err + } /* TODO: MultiEd25519 is not supported ATM TestSigners["MultiEd25519"] = func() (TransactionSigner, error) { signer, err := NewMultiEd25519Signer(3, 2) diff --git a/crypto/multiKey.go b/crypto/multiKey.go index db165df..d6402bc 100644 --- a/crypto/multiKey.go +++ b/crypto/multiKey.go @@ -349,33 +349,56 @@ func (ea *MultiKeyAuthenticator) UnmarshalBCS(des *bcs.Deserializer) { //region MultiKeyBitmap -// MultiKeyBitmapSize represents the 4 bytes needed to make a 32-bit bitmap -const MultiKeyBitmapSize = uint8(4) +// MaxMultiKeySignatures is the maximum number supported here for ease of use +const MaxMultiKeySignatures = uint8(32) +const MaxMultiKeyBitmapBytes = MaxMultiKeySignatures / 8 // MultiKeyBitmap represents a bitmap of signatures in a MultiKey public key that signed the transaction -// There are a maximum of 32 possible values in MultiKeyBitmapSize, starting from the leftmost bit representing +// There is a variable length to the possible signers, starting from the leftmost bit representing // index 0 of the public key -type MultiKeyBitmap [MultiKeyBitmapSize]byte +type MultiKeyBitmap struct { + inner []byte +} // ContainsKey tells us if the current index is in the map func (bm *MultiKeyBitmap) ContainsKey(index uint8) bool { + if index >= MaxMultiKeySignatures { + return false + } numByte, numBit := KeyIndices(index) - return (bm[numByte] & (128 >> numBit)) == 1 + + // If it's outside the map, then it's definitely false + if int(numByte) >= len(bm.inner) { + return false + } + return (bm.inner[numByte] & (128 >> numBit)) == 1 } // AddKey adds the value to the map, returning an error if it is already added func (bm *MultiKeyBitmap) AddKey(index uint8) error { + if index >= MaxMultiKeySignatures { + return fmt.Errorf("index %d is greater than the maximum number of signatures %d", index, MaxMultiKeySignatures) + } if bm.ContainsKey(index) { return fmt.Errorf("index %d already in bitmap", index) } numByte, numBit := KeyIndices(index) - bm[numByte] = bm[numByte] | (128 >> numBit) + + // Expand map if it needs to be extended + curLength := len(bm.inner) + if int(numByte) >= curLength { + newInner := make([]byte, numByte+1) + copy(newInner[0:curLength], bm.inner[:]) + bm.inner = newInner + } + + bm.inner[numByte] = bm.inner[numByte] | (128 >> numBit) return nil } func (bm *MultiKeyBitmap) Indices() []uint8 { indices := make([]uint8, 0) - for i := uint8(0); i < MultiKeyBitmapSize*8; i++ { + for i := uint8(0); i < MaxMultiKeySignatures; i++ { if bm.ContainsKey(i) { indices = append(indices, i) } @@ -396,7 +419,7 @@ func KeyIndices(index uint8) (numByte uint8, numBit uint8) { // Implements: // - [bcs.Marshaler] func (bm *MultiKeyBitmap) MarshalBCS(ser *bcs.Serializer) { - ser.WriteBytes(bm[:]) + ser.WriteBytes(bm.inner[:]) } // UnmarshalBCS deserializes the bitmap from bytes @@ -405,11 +428,12 @@ func (bm *MultiKeyBitmap) MarshalBCS(ser *bcs.Serializer) { // - [bcs.Unmarshaler] func (bm *MultiKeyBitmap) UnmarshalBCS(des *bcs.Deserializer) { length := des.Uleb128() - if length != uint32(MultiKeyBitmapSize) { - des.SetError(fmt.Errorf("MultiKeyBitmap must be %d bytes, got %d", MultiKeyBitmapSize, length)) + if length > uint32(MaxMultiKeyBitmapBytes) { + des.SetError(fmt.Errorf("MultiKeyBitmap must be %d bytes or less, got %d", MaxMultiKeyBitmapBytes, length)) return } - des.ReadFixedBytesInto(bm[:]) + bm.inner = make([]byte, length) + des.ReadFixedBytesInto(bm.inner[:]) } //endregion diff --git a/crypto/multiKey_test.go b/crypto/multiKey_test.go index 1912f28..1c5c229 100644 --- a/crypto/multiKey_test.go +++ b/crypto/multiKey_test.go @@ -1,6 +1,7 @@ package crypto import ( + "encoding/hex" "github.com/aptos-labs/aptos-go-sdk/bcs" "github.com/stretchr/testify/assert" "testing" @@ -85,6 +86,18 @@ func TestMultiKeySerialization(t *testing.T) { } +func TestMultiKey_Serialization_CrossPlatform(t *testing.T) { + serialized := "020140118d6ebe543aaf3a541453f98a5748ab5b9e3f96d781b8c0a43740af2b65c03529fdf62b7de7aad9150770e0994dc4e0714795fdebf312be66cd0550c607755e00401a90421453aa53fa5a7aa3dfe70d913823cbf087bf372a762219ccc824d3a0eeecccaa9d34f22db4366aec61fb6c204d2440f4ed288bc7cc7e407b766723a60901c0" + serializedBytes, err := hex.DecodeString(serialized) + assert.NoError(t, err) + signature := &MultiKeySignature{} + assert.NoError(t, bcs.Deserialize(signature, serializedBytes)) + + reserialized, err := bcs.Serialize(signature) + assert.NoError(t, err) + assert.Equal(t, serializedBytes, reserialized) +} + func createMultiKey(t *testing.T) ( *SingleSigner, *SingleSigner, diff --git a/signer_test.go b/signer_test.go index b018ed6..d7cf3f6 100644 --- a/signer_test.go +++ b/signer_test.go @@ -2,6 +2,7 @@ package aptos import ( "github.com/aptos-labs/aptos-go-sdk/crypto" + "math/rand/v2" ) /* This is a collection of test signers, that don't make sense in the real world, but are used for testing */ @@ -91,7 +92,7 @@ type MultiKeyTestSigner struct { func NewMultiKeyTestSigner(numKeys uint8, signaturesRequired uint8) (*MultiKeyTestSigner, error) { signers := make([]crypto.Signer, numKeys) for i := range signers { - switch i % 3 { + switch i % 2 { case 0: signer, err := crypto.GenerateEd25519PrivateKey() if err != nil { @@ -99,7 +100,7 @@ func NewMultiKeyTestSigner(numKeys uint8, signaturesRequired uint8) (*MultiKeyTe } // Wrap in a SingleSigner signers[i] = &crypto.SingleSigner{Signer: signer} - case 1, 2: + case 1: signer, err := crypto.GenerateSecp256k1Key() if err != nil { return nil, err @@ -150,16 +151,25 @@ func (s *MultiKeyTestSigner) Sign(msg []byte) (authenticator *crypto.AccountAuth func (s *MultiKeyTestSigner) SignMessage(msg []byte) (crypto.Signature, error) { indexedSigs := make([]crypto.IndexedAnySignature, s.SignaturesRequired) - // TODO: randomize keys used for testing + alreadyUsed := make(map[int]bool) + for i := uint8(0); i < s.SignaturesRequired; i++ { - // We skip over keys to ensure that order doesn't matter - signerIndex := (i * 2) % uint8(len(s.Signers)) + // Find a random key + index := 0 + for { + index = rand.IntN(len(s.Signers)) + _, present := alreadyUsed[index] + if !present { + alreadyUsed[index] = true + break + } + } - sig, err := s.Signers[signerIndex].SignMessage(msg) + sig, err := s.Signers[index].SignMessage(msg) if err != nil { return nil, err } - indexedSigs[i] = crypto.IndexedAnySignature{Signature: sig.(*crypto.AnySignature), Index: signerIndex} + indexedSigs[i] = crypto.IndexedAnySignature{Signature: sig.(*crypto.AnySignature), Index: uint8(index)} } return crypto.NewMultiKeySignature(indexedSigs)