Skip to content

Conversation

@id-ms
Copy link
Contributor

@id-ms id-ms commented Oct 3, 2022

Summary

Add several unit tests for verifying types of signatures on transactions. including: regular signature, multisignature and logicsignature

Test Plan

@id-ms id-ms changed the title Testing: extend the testing on transactions signature verification Tests: extend the testing on transactions signature verification Oct 3, 2022
@id-ms id-ms changed the title Tests: extend the testing on transactions signature verification testing: extend the testing on transactions signature verification Oct 3, 2022
@codecov
Copy link

codecov bot commented Oct 3, 2022

Codecov Report

Merging #4623 (0f2aa60) into master (b4fecd5) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4623      +/-   ##
==========================================
+ Coverage   54.27%   54.33%   +0.05%     
==========================================
  Files         402      402              
  Lines       51809    51809              
==========================================
+ Hits        28119    28148      +29     
+ Misses      21330    21284      -46     
- Partials     2360     2377      +17     
Impacted Files Coverage Δ
ledger/voters.go 68.65% <0.00%> (-4.48%) ⬇️
ledger/blockqueue.go 84.48% <0.00%> (-4.03%) ⬇️
ledger/tracker.go 74.04% <0.00%> (-3.83%) ⬇️
network/wsPeer.go 65.50% <0.00%> (-2.68%) ⬇️
crypto/merkletrie/trie.go 66.42% <0.00%> (-2.19%) ⬇️
crypto/merkletrie/node.go 91.62% <0.00%> (-1.87%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
ledger/acctonline.go 77.60% <0.00%> (-0.53%) ⬇️
catchup/service.go 69.38% <0.00%> (+0.74%) ⬆️
agreement/proposalManager.go 98.03% <0.00%> (+1.96%) ⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@id-ms id-ms changed the title testing: extend the testing on transactions signature verification testing: extend transactions signature verification testing Oct 3, 2022
Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

Looks reasonable, but please explain the logic sig program in the last test

@id-ms id-ms force-pushed the sig-verification-tests branch from 1701a58 to 4b413c6 Compare October 3, 2022 13:26
algonautshant
algonautshant previously approved these changes Oct 4, 2022
Copy link
Contributor

@algonautshant algonautshant left a comment

Choose a reason for hiding this comment

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

Look Great!
This is very nicely structured, and can easily adapted for other tests.
Just one minor comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants