diff --git a/core/v3/signature_build_test.go b/core/v3/signature_build_test.go new file mode 100644 index 00000000..cfe66f9d --- /dev/null +++ b/core/v3/signature_build_test.go @@ -0,0 +1,90 @@ +package v3_test + +import ( + "bytes" + "context" + "math/big" + "testing" + + "github.com/0xsequence/ethkit/go-ethereum/common" + "github.com/0xsequence/go-sequence/core" + v3 "github.com/0xsequence/go-sequence/core/v3" + "github.com/stretchr/testify/require" +) + +// A subdigest leaf reports max signersWeight for any payload, so the config threshold +// looks met as soon as the first signature is collected. Early cancellation on that +// estimate nondeterministically drops the other signer's signature even though recovery +// of a payload not matching the subdigest still needs it. Every build must embed both +// signatures and produce identical bytes. +func TestBuildRegularSignatureCollectsAllSignersDespiteSubdigestLeaf(t *testing.T) { + signerA := common.HexToAddress("0x1111111111111111111111111111111111111111") + signerB := common.HexToAddress("0x2222222222222222222222222222222222222222") + + dummySignature := func(fill byte) []byte { + sig := make([]byte, 65) + for i := range 64 { + sig[i] = fill + } + sig[64] = 27 + return sig + } + signatureA := dummySignature(0xaa) + signatureB := dummySignature(0xbb) + + config := &v3.WalletConfig{ + Threshold_: 2, + Tree: v3.WalletConfigTreeNodes( + v3.WalletConfigTreeSubdigestLeaf{Subdigest: common.BigToHash(big.NewInt(1))}, + &v3.WalletConfigTreeAddressLeaf{Weight: 1, Address: signerA}, + &v3.WalletConfigTreeAddressLeaf{Weight: 1, Address: signerB}, + ), + } + + signatures := map[common.Address][]byte{ + signerA: signatureA, + signerB: signatureB, + } + signingFunc := func(ctx context.Context, signer core.Signer, _ []core.SignerSignature) (core.SignerSignatureType, []byte, error) { + if sig, ok := signatures[signer.Address]; ok { + return core.SignerSignatureTypeEthSign, sig, nil + } + return 0, nil, nil + } + + var first []byte + for range 100 { + sig, err := config.BuildRegularSignature(context.Background(), signingFunc, true) + require.NoError(t, err) + data, err := sig.Data() + require.NoError(t, err) + require.True(t, bytes.Contains(data, signatureA[:64]), "signer A's signature must be embedded on every build") + require.True(t, bytes.Contains(data, signatureB[:64]), "signer B's signature must be embedded on every build") + if first == nil { + first = data + } + require.Equal(t, first, data, "signature encoding must be deterministic") + } +} + +// A subdigest leaf reports max signersWeight even for an empty signer set, so +// signing-power validation must still fail when no signature is collected at all. +func TestBuildSignatureValidationRejectsEmptySignerSetDespiteSubdigestLeaf(t *testing.T) { + config := &v3.WalletConfig{ + Threshold_: 2, + Tree: v3.WalletConfigTreeNodes( + v3.WalletConfigTreeSubdigestLeaf{Subdigest: common.BigToHash(big.NewInt(1))}, + &v3.WalletConfigTreeAddressLeaf{Weight: 1, Address: common.HexToAddress("0x1111111111111111111111111111111111111111")}, + ), + } + + signingFunc := func(ctx context.Context, signer core.Signer, _ []core.SignerSignature) (core.SignerSignatureType, []byte, error) { + return 0, nil, nil + } + + _, err := config.BuildRegularSignature(context.Background(), signingFunc, true) + require.ErrorContains(t, err, "not enough signers") + + _, err = config.BuildNoChainIDSignature(context.Background(), signingFunc, true) + require.ErrorContains(t, err, "not enough signers") +} diff --git a/core/v3/v3.go b/core/v3/v3.go index e9574498..3f172186 100644 --- a/core/v3/v3.go +++ b/core/v3/v3.go @@ -2001,6 +2001,7 @@ func (c *WalletConfig) BuildSubdigestSignature(noChainID bool) (core.Signature[* func (c *WalletConfig) BuildRegularSignature(ctx context.Context, sign core.SigningFunction, validateSigningPower bool, checkpointerData ...[]byte) (core.Signature[*WalletConfig], error) { var isValid bool configSigners := c.Signers() + threshold := new(big.Int).SetUint64(uint64(c.Threshold_)) signCtx, signCancel := context.WithCancel(ctx) defer signCancel() @@ -2020,9 +2021,15 @@ func (c *WalletConfig) BuildRegularSignature(ctx context.Context, sign core.Sign signerSignatures[signerSignature.Signer] = signerSignature signedSigners[signerSignature.Signer] = configSigners[signerSignature.Signer] - weight := c.Tree.signersWeight(signedSigners) - if weight.Cmp(new(big.Int).SetUint64(uint64(c.Threshold_))) >= 0 { + // Cancel outstanding signers only once collected signatures alone meet the + // threshold. signersWeight counts payload-blind subdigest leaves as satisfied, + // and cancelling on that estimate nondeterministically drops signatures that + // recovery of a non-matching payload still needs. + if c.Tree.collectedSignersWeight(signedSigners).Cmp(threshold) >= 0 { signCancel() + } + + if c.Tree.signersWeight(signedSigners).Cmp(threshold) >= 0 { isValid = true } } @@ -2050,6 +2057,7 @@ func (c *WalletConfig) BuildRegularSignature(ctx context.Context, sign core.Sign func (c *WalletConfig) BuildNoChainIDSignature(ctx context.Context, sign core.SigningFunction, validateSigningPower bool, checkpointerData ...[]byte) (core.Signature[*WalletConfig], error) { var isValid bool configSigners := c.Signers() + threshold := new(big.Int).SetUint64(uint64(c.Threshold_)) signCtx, signCancel := context.WithCancel(ctx) defer signCancel() @@ -2069,9 +2077,15 @@ func (c *WalletConfig) BuildNoChainIDSignature(ctx context.Context, sign core.Si signerSignatures[signerSignature.Signer] = signerSignature signedSigners[signerSignature.Signer] = configSigners[signerSignature.Signer] - weight := c.Tree.signersWeight(signedSigners) - if weight.Cmp(new(big.Int).SetUint64(uint64(c.Threshold_))) >= 0 { + // Cancel outstanding signers only once collected signatures alone meet the + // threshold. signersWeight counts payload-blind subdigest leaves as satisfied, + // and cancelling on that estimate nondeterministically drops signatures that + // recovery of a non-matching payload still needs. + if c.Tree.collectedSignersWeight(signedSigners).Cmp(threshold) >= 0 { signCancel() + } + + if c.Tree.signersWeight(signedSigners).Cmp(threshold) >= 0 { isValid = true } } @@ -2126,6 +2140,11 @@ type WalletConfigTree interface { isComplete() bool maxWeight() *big.Int signersWeight(signers map[core.Signer]uint16) *big.Int + // collectedSignersWeight is signersWeight with payload-blind leaves (subdigest + // leaves, which claim max weight for any payload) counting zero, so it only + // reflects weight from actually collected signatures. Safe to use for early + // cancellation decisions; signersWeight is not. + collectedSignersWeight(signers map[core.Signer]uint16) *big.Int readSignersIntoMap(signers map[core.Signer]uint16) buildSignatureTree(signerSignatures map[core.Signer]core.SignerSignature) signatureTree } @@ -2291,6 +2310,10 @@ func (n *WalletConfigTreeNode) signersWeight(signers map[core.Signer]uint16) *bi return new(big.Int).Add(n.Left.signersWeight(signers), n.Right.signersWeight(signers)) } +func (n *WalletConfigTreeNode) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return new(big.Int).Add(n.Left.collectedSignersWeight(signers), n.Right.collectedSignersWeight(signers)) +} + func (n *WalletConfigTreeNode) readSignersIntoMap(signers map[core.Signer]uint16) { n.Left.readSignersIntoMap(signers) n.Right.readSignersIntoMap(signers) @@ -2369,6 +2392,10 @@ func (l *WalletConfigTreeAddressLeaf) signersWeight(signers map[core.Signer]uint } } +func (l *WalletConfigTreeAddressLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return l.signersWeight(signers) +} + func (l *WalletConfigTreeAddressLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { signers[core.Signer{Address: l.Address}] = uint16(l.Weight) } @@ -2466,6 +2493,10 @@ func (l WalletConfigTreeNodeLeaf) signersWeight(signers map[core.Signer]uint16) return new(big.Int) } +func (l WalletConfigTreeNodeLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return new(big.Int) +} + func (l WalletConfigTreeNodeLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { } @@ -2553,6 +2584,13 @@ func (l *WalletConfigTreeNestedLeaf) signersWeight(signers map[core.Signer]uint1 return new(big.Int) } +func (l *WalletConfigTreeNestedLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + if l.Tree.collectedSignersWeight(signers).Cmp(new(big.Int).SetUint64(uint64(l.Threshold))) >= 0 { + return new(big.Int).SetUint64(uint64(l.Weight)) + } + return new(big.Int) +} + func (l *WalletConfigTreeNestedLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { l.Tree.readSignersIntoMap(signers) } @@ -2619,6 +2657,10 @@ func (l WalletConfigTreeSubdigestLeaf) signersWeight(signers map[core.Signer]uin return new(big.Int).Set(maxUint256) } +func (l WalletConfigTreeSubdigestLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return new(big.Int) +} + func (l WalletConfigTreeSubdigestLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { } @@ -2709,6 +2751,10 @@ func (l *WalletConfigTreeSapientSignerLeaf) signersWeight(signers map[core.Signe } } +func (l *WalletConfigTreeSapientSignerLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return l.signersWeight(signers) +} + func (l *WalletConfigTreeSapientSignerLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { signers[core.SapientSigner(l.Address, l.ImageHash_.Hash)] = uint16(l.Weight) } @@ -2785,6 +2831,10 @@ func (l WalletConfigTreeAnyAddressSubdigestLeaf) signersWeight(signers map[core. return new(big.Int).Set(maxUint256) } +func (l WalletConfigTreeAnyAddressSubdigestLeaf) collectedSignersWeight(signers map[core.Signer]uint16) *big.Int { + return new(big.Int) +} + func (l WalletConfigTreeAnyAddressSubdigestLeaf) readSignersIntoMap(signers map[core.Signer]uint16) { }