Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BatchVerifier: Rename and unexport local functions in verify/txn #4578

Merged
merged 13 commits into from
Sep 30, 2022
Merged

BatchVerifier: Rename and unexport local functions in verify/txn #4578

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3608cf9
rename and unexport local functions
algonautshant Sep 23, 2022
05d63ae
update the tests
algonautshant Sep 23, 2022
58e4d3f
update a test
algonautshant Sep 23, 2022
1e96dd6
update test err msg
algonautshant Sep 23, 2022
2db4ee5
CR updates
algonautshant Sep 26, 2022
b0ef3a0
add test and refactoring
algonautshant Sep 27, 2022
530e3a3
Merge remote-tracking branch 'origin/shant/verify_renamings' into sha…
algonautshant Sep 27, 2022
caca719
CR comments
algonautshant Sep 27, 2022
754f0fb
cache txn when only logicsig
algonautshant Sep 28, 2022
d2f8e16
fix comment
algonautshant Sep 28, 2022
dffe43e
renaming per CR
algonautshant Sep 28, 2022
8f5df56
cleanups requested in CR
algonautshant Sep 28, 2022
9ff7853
CR comments: rename and drop enqueued sig count
algonautshant Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions crypto/batchverifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const minBatchVerifierAlloc = 16
// Batch verifications errors
var (
ErrBatchVerificationFailed = errors.New("At least one signature didn't pass verification")
ErrZeroTransactionInBatch = errors.New("Could not validate empty signature set")
)

//export ed25519_randombytes_unsafe
Expand Down Expand Up @@ -113,7 +112,7 @@ func (b *BatchVerifier) GetNumberOfEnqueuedSignatures() int {
// if the batch is zero an appropriate error is return.
func (b *BatchVerifier) Verify() error {
if b.GetNumberOfEnqueuedSignatures() == 0 {
return ErrZeroTransactionInBatch
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change means that TxnGroup will now add things to the cache where before it would just exit out early with the ctx and nil error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is the intention, so that the logicsig transactions which were not added to BatchVerifier at the level of TxnGroup and do not contribute to this GetNumberOfEnqueuedSignatures, but were verified at local batches, also get cached.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that too, I looked up the version just before the BV refactor merged in #2578 and it looks like any verified txngroup (even those with 0 sigs) would get cached in the old version
https://github.com/algorand/go-algorand/blame/16bada1e2b185348a41e1590eefd9eebd4315064/data/transactions/verify/txn.go#L115

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so are we OK this this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we discussed it and it is the intention to make this change and then have logicsigs be cached like they used to.

}

var messages = make([][]byte, b.GetNumberOfEnqueuedSignatures())
Expand Down
2 changes: 1 addition & 1 deletion crypto/batchverifier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,5 @@ func BenchmarkBatchVerifier(b *testing.B) {
func TestEmpty(t *testing.T) {
partitiontest.PartitionTest(t)
bv := MakeBatchVerifier()
require.Error(t, bv.Verify())
require.NoError(t, bv.Verify())
}
6 changes: 3 additions & 3 deletions crypto/multisig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func MultisigAssemble(unisig []MultisigSig) (msig MultisigSig, err error) {
func MultisigVerify(msg Hashable, addr Digest, sig MultisigSig) (verified bool, err error) {
batchVerifier := MakeBatchVerifier()

if verified, err = MultisigBatchVerify(msg, addr, sig, batchVerifier); err != nil {
if verified, err = MultisigBatchPrep(msg, addr, sig, batchVerifier); err != nil {
algonautshant marked this conversation as resolved.
Show resolved Hide resolved
return
}
if !verified {
Expand All @@ -234,9 +234,9 @@ func MultisigVerify(msg Hashable, addr Digest, sig MultisigSig) (verified bool,
return true, nil
}

// MultisigBatchVerify verifies an assembled MultisigSig.
// MultisigBatchPrep verifies an assembled MultisigSig.
// it is the caller responsibility to call batchVerifier.verify()
func MultisigBatchVerify(msg Hashable, addr Digest, sig MultisigSig, batchVerifier *BatchVerifier) (verified bool, err error) {
func MultisigBatchPrep(msg Hashable, addr Digest, sig MultisigSig, batchVerifier *BatchVerifier) (verified bool, err error) {
algonautshant marked this conversation as resolved.
Show resolved Hide resolved
verified = false
// short circuit: if msig doesn't have subsigs or if Subsigs are empty
// then terminate (the upper layer should now verify the unisig)
Expand Down
12 changes: 6 additions & 6 deletions crypto/multisig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestMultisig(t *testing.T) {

//test3: use the batch verification
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, msig, br)
verify, err = MultisigBatchPrep(txid, addr, msig, br)
require.NoError(t, err, "Multisig: unexpected verification failure with err")
require.True(t, verify, "Multisig: verification failed, verify flag was false")
res := br.Verify()
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestEmptyMultisig(t *testing.T) {
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, emptyMutliSig, br)
verify, err = MultisigBatchPrep(txid, addr, emptyMutliSig, br)
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
}
Expand Down Expand Up @@ -286,7 +286,7 @@ func TestIncorrectAddrresInMultisig(t *testing.T) {
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, MutliSig, br)
verify, err = MultisigBatchPrep(txid, addr, MutliSig, br)
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")

Expand Down Expand Up @@ -325,7 +325,7 @@ func TestMoreThanMaxSigsInMultisig(t *testing.T) {
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, msig, br)
verify, err = MultisigBatchPrep(txid, addr, msig, br)
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
}
Expand Down Expand Up @@ -364,7 +364,7 @@ func TestOneSignatureIsEmpty(t *testing.T) {
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, msig, br)
verify, err = MultisigBatchPrep(txid, addr, msig, br)
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
}
Expand Down Expand Up @@ -405,7 +405,7 @@ func TestOneSignatureIsInvalid(t *testing.T) {
require.False(t, verify, "Multisig: verification succeeded, it should failed")
require.Error(t, err, "Multisig: did not return error as expected")
br := MakeBatchVerifier()
verify, err = MultisigBatchVerify(txid, addr, msig, br)
verify, err = MultisigBatchPrep(txid, addr, msig, br)
require.NoError(t, err, "Multisig: did not return error as expected")
require.True(t, verify, "Multisig: verification succeeded, it should failed")
res := br.Verify()
Expand Down
70 changes: 25 additions & 45 deletions data/transactions/verify/txn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,59 +99,43 @@ func (g *GroupContext) Equal(other *GroupContext) bool {
g.minAvmVersion == other.minAvmVersion
}

// Txn verifies a SignedTxn as being signed and having no obviously inconsistent data.
algorandskiy marked this conversation as resolved.
Show resolved Hide resolved
// txnBatchPrep verifies a SignedTxn having no obviously inconsistent data.
// Block-assembly time checks of LogicSig and accounting rules may still block the txn.
func Txn(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext) error {
batchVerifier := crypto.MakeBatchVerifier()

if err := TxnBatchVerify(s, txnIdx, groupCtx, batchVerifier); err != nil {
return err
}

// this case is used for comapact certificate where no signature is supplied
if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 {
return nil
}
return batchVerifier.Verify()
}

// TxnBatchVerify verifies a SignedTxn having no obviously inconsistent data.
// Block-assembly time checks of LogicSig and accounting rules may still block the txn.
// it is the caller responsibility to call batchVerifier.verify()
func TxnBatchVerify(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) error {
// it is the caller responsibility to call batchVerifier.Verify()
func txnBatchPrep(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) error {
if !groupCtx.consensusParams.SupportRekeying && (s.AuthAddr != basics.Address{}) {
return errors.New("nonempty AuthAddr but rekeying not supported")
return errors.New("nonempty AuthAddr but rekeying is not supported")
}

if err := s.Txn.WellFormed(groupCtx.specAddrs, groupCtx.consensusParams); err != nil {
return err
}

return stxnVerifyCore(s, txnIdx, groupCtx, verifier)
return stxnCoreChecks(s, txnIdx, groupCtx, verifier)
}

// TxnGroup verifies a []SignedTxn as being signed and having no obviously inconsistent data.
func TxnGroup(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature) (groupCtx *GroupContext, err error) {
batchVerifier := crypto.MakeBatchVerifier()

if groupCtx, err = TxnGroupBatchVerify(stxs, contextHdr, cache, ledger, batchVerifier); err != nil {
if groupCtx, err = txnGroupBatchPrep(stxs, contextHdr, ledger, batchVerifier); err != nil {
return nil, err
}

algonautshant marked this conversation as resolved.
Show resolved Hide resolved
if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 {
return groupCtx, nil
}

if err := batchVerifier.Verify(); err != nil {
return nil, err
}

if cache != nil {
cache.Add(stxs, groupCtx)
}

return
}

// TxnGroupBatchVerify verifies a []SignedTxn having no obviously inconsistent data.
// it is the caller responsibility to call batchVerifier.verify()
func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature, verifier *crypto.BatchVerifier) (groupCtx *GroupContext, err error) {
// txnGroupBatchPrep verifies a []SignedTxn having no obviously inconsistent data.
// it is the caller responsibility to call batchVerifier.Verify()
func txnGroupBatchPrep(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, ledger logic.LedgerForSignature, verifier *crypto.BatchVerifier) (groupCtx *GroupContext, err error) {
groupCtx, err = PrepareGroupContext(stxs, contextHdr, ledger)
if err != nil {
return nil, err
Expand All @@ -160,7 +144,7 @@ func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.B
minFeeCount := uint64(0)
feesPaid := uint64(0)
for i, stxn := range stxs {
err = TxnBatchVerify(&stxn, i, groupCtx, verifier)
err = txnBatchPrep(&stxn, i, groupCtx, verifier)
if err != nil {
err = fmt.Errorf("transaction %+v invalid : %w", stxn, err)
return
Expand All @@ -184,13 +168,10 @@ func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.B
return
}

if cache != nil {
cache.Add(stxs, groupCtx)
}
return
}

func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error {
func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error {
numSigs := 0
hasSig := false
hasMsig := false
Expand Down Expand Up @@ -227,7 +208,7 @@ func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex
return nil
}
if hasMsig {
if ok, _ := crypto.MultisigBatchVerify(s.Txn,
if ok, _ := crypto.MultisigBatchPrep(s.Txn,
crypto.Digest(s.Authorizer()),
s.Msig,
batchVerifier); ok {
Expand All @@ -236,7 +217,7 @@ func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex
return errors.New("multisig validation failed")
}
if hasLogicSig {
return logicSigBatchVerify(s, txnIdx, groupCtx)
return logicSigVerify(s, txnIdx, groupCtx)
}
return errors.New("has one mystery sig. WAT?")
}
Expand All @@ -246,7 +227,7 @@ func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex
func LogicSigSanityCheck(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error {
batchVerifier := crypto.MakeBatchVerifier()

if err := LogicSigSanityCheckBatchVerify(txn, groupIndex, groupCtx, batchVerifier); err != nil {
if err := logicSigSanityCheckBatchPrep(txn, groupIndex, groupCtx, batchVerifier); err != nil {
return err
}

Expand All @@ -258,10 +239,10 @@ func LogicSigSanityCheck(txn *transactions.SignedTxn, groupIndex int, groupCtx *
return batchVerifier.Verify()
}

// LogicSigSanityCheckBatchVerify checks that the signature is valid and that the program is basically well formed.
// logicSigSanityCheckBatchPrep checks that the signature is valid and that the program is basically well formed.
// It does not evaluate the logic.
// it is the caller responsibility to call batchVerifier.verify()
func LogicSigSanityCheckBatchVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error {
// it is the caller responsibility to call batchVerifier.Verify()
func logicSigSanityCheckBatchPrep(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error {
lsig := txn.Lsig

if groupCtx.consensusParams.LogicSigVersion == 0 {
Expand Down Expand Up @@ -323,16 +304,15 @@ func LogicSigSanityCheckBatchVerify(txn *transactions.SignedTxn, groupIndex int,
batchVerifier.EnqueueSignature(crypto.PublicKey(txn.Authorizer()), &program, lsig.Sig)
} else {
program := logic.Program(lsig.Logic)
if ok, _ := crypto.MultisigBatchVerify(&program, crypto.Digest(txn.Authorizer()), lsig.Msig, batchVerifier); !ok {
if ok, _ := crypto.MultisigBatchPrep(&program, crypto.Digest(txn.Authorizer()), lsig.Msig, batchVerifier); !ok {
return errors.New("logic multisig validation failed")
}
}
return nil
}

// logicSigBatchVerify checks that the signature is valid, executing the program.
// it is the caller responsibility to call batchVerifier.verify()
func logicSigBatchVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error {
// logicSigVerify checks that the signature is valid, executing the program.
func logicSigVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error {
algonautshant marked this conversation as resolved.
Show resolved Hide resolved
err := LogicSigSanityCheck(txn, groupIndex, groupCtx)
if err != nil {
return err
Expand Down Expand Up @@ -404,7 +384,7 @@ func PaysetGroups(ctx context.Context, payset [][]transactions.SignedTxn, blkHea

batchVerifier := crypto.MakeBatchVerifierWithHint(len(payset))
for i, signTxnsGrp := range txnGroups {
groupCtxs[i], grpErr = TxnGroupBatchVerify(signTxnsGrp, blkHeader, nil, ledger, batchVerifier)
groupCtxs[i], grpErr = txnGroupBatchPrep(signTxnsGrp, blkHeader, ledger, batchVerifier)
// abort only if it's a non-cache error.
if grpErr != nil {
return grpErr
Expand Down
Loading