AVM: SHA3-256 hash function opcode #3544

Matt-Ryley · 2022-02-01T10:25:00Z

Implemented the SHA3-256 hash function required for interoperability of Algorand with the ICON blockchain.
Added the OPCode 0x96 for this functionality.
Updated documentation and unit tests to include the new SHA3-256 hashing function.

Added SHA3-256 hashing function as an OP code

Made required changes to pass the unit tests. -- Changed testAccepts version from 1 to 6 -- Added sha3_256 to the Opgroups

From code review suggestion renamed opSHA3256 to opSHA3_256 in the eval.go and opcodes.go file

Sha3 256

CLAassistant · 2022-02-01T10:25:07Z

All committers have signed the CLA.

jannotti · 2022-02-01T15:07:09Z

data/transactions/logic/opcodes.go

@@ -294,6 +294,9 @@ var OpSpecs = []OpSpec{
 	{0x94, "exp", opExp, asmDefault, disDefault, twoInts, oneInt, 4, modeAny, opDefault},
 	{0x95, "expw", opExpw, asmDefault, disDefault, twoInts, twoInts, 4, modeAny, costly(10)},

+	//More Hash Functions
+	{0x96, "sha3_256", opSHA3_256, asmDefault, disDefault, oneBytes, oneBytes, 6, modeAny, costly(130)},


How did you derive the cost?

I wasn't sure how to derive the cost. The cost of the SHA3-256 should be roughly the same as the Keccak256 as they are very similar algorithms, except the padding in the SHA3-256 adds up to 4 additional bits. Im open to any suggestions of how to derive this cost more accurately?

There is some benchmarking code in eval_test.go. But hold off for a couple days. We're in the middle of a merge that will change it slightly. But once that merge happens, you'll find
func BenchmarkBase64Decode(b *testing.B) that was used to compare base64 decode to the costs of various hashes. By adding sha3_256 in there, we should be able to get a feel for relative costs.

(The function may already be in master, if you want to look now.)

Thanks for the information! Which merge should I be looking out for?

feature/contract-to-contract going into master #3397

hey @Matt-Ryley, just FYI the aforementioned merge happened.

Thanks for the heads up @algoanne! Ill get on this as soon as possible

I have just ran the sha3_256 through the suggested function and here is the output, comparing keccak256 and sha3_256
BenchmarkBase64Decode/keccak256_small-8 518492 2221 ns/op
BenchmarkBase64Decode/sha3_256_small-8 525247 1957 ns/op

To me they are in the same ball park as one another and thus should have the same cost, as the eqv for other hashes like
BenchmarkBase64Decode/sha256_small-8 1260788 1003 ns/op
BenchmarkBase64Decode/sha512_256_small-8 1695235 715.2 ns/op

Are very different

Yes, it sounds like it should have the same cost as keccak. I think we can take this as soon as CI passes. I'll be making a pass over it once that happens as I have changed the opcode interface a bit. But I think your work is done! Thanks for the contribution.

Spelling mistake rectified

jannotti · 2022-03-10T21:50:28Z

Sorry, it looks like one thing you'll have to do to make CI happy is run make in the data/transactions/logic directory. That will regenerate a couple spec files from your docstrings. That's the codegen_verification step that's failing.

Didnt mean to push them!

accidentally pushed all local changes!

codecov-commenter · 2022-03-11T12:33:32Z

Codecov Report

Merging #3544 (5d8d05d) into master (5a72986) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3544      +/-   ##
==========================================
+ Coverage   49.57%   49.60%   +0.02%     
==========================================
  Files         392      392              
  Lines       68590    68596       +6     
==========================================
+ Hits        34006    34024      +18     
+ Misses      30845    30836       -9     
+ Partials     3739     3736       -3

Impacted Files	Coverage Δ
data/transactions/logic/doc.go	`56.00% <ø> (ø)`
data/transactions/logic/opcodes.go	`100.00% <ø> (ø)`
data/transactions/logic/eval.go	`86.07% <100.00%> (+0.03%)`	⬆️
data/abi/abi_type.go	`87.67% <0.00%> (-0.95%)`	⬇️
network/requestTracker.go	`70.25% <0.00%> (-0.44%)`	⬇️
ledger/acctupdates.go	`68.42% <0.00%> (+0.26%)`	⬆️
network/wsPeer.go	`69.16% <0.00%> (+0.83%)`	⬆️
data/transactions/verify/txn.go	`45.02% <0.00%> (+0.86%)`	⬆️
ledger/blockqueue.go	`85.05% <0.00%> (+2.87%)`	⬆️
ledger/roundlru.go	`96.22% <0.00%> (+5.66%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a72986...5d8d05d. Read the comment docs.

Matt-Ryley · 2022-03-14T11:04:24Z

@jannotti after upstream merge the arm64 test is now failing in one of the tests for the ledger. As I haven't modified any code in that directory I am confused to why this test fails. Do you know of any reason? My local machine is amd64 architecture and those tests run successfully in the CI pipeline and also on my local machine, so I can't really debug to find the cause of the error.

jannotti · 2022-03-14T14:08:07Z

I'm not sure, but I'll be looking at it more closely today. I used a new PR, so I would be able to tweak it (to handle merge conflicts). #3762 I believe I merged your commits in properly so you will retain credit. Thanks again.

…

On Mon, Mar 14, 2022 at 7:04 AM Matt-Ryley ***@***.***> wrote: @jannotti <https://github.com/jannotti> after upstream merge the arm64 test is now failing in one of the tests for the ledger. As I haven't modified any code in that directory I am confused to why this test fails. Do you know of any reason? My local machine is amd64 architecture and those tests run successfully in the CI pipeline and also on my local machine, so I can't really debug to find the cause of the error. — Reply to this email directly, view it on GitHub <#3544 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADL7T6MTB7OTAMTHE5ESADU74MMHANCNFSM5NIXB5AQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jannotti · 2022-03-14T14:08:33Z

Closed in favor of #3762

Matt-Ryley added 5 commits January 27, 2022 16:04

SHA3-256 added

7b4a194

Added SHA3-256 hashing function as an OP code

Unit tests now pass

510b04d

Made required changes to pass the unit tests. -- Changed testAccepts version from 1 to 6 -- Added sha3_256 to the Opgroups

SHA3256->SHA3_256

fff5189

From code review suggestion renamed opSHA3256 to opSHA3_256 in the eval.go and opcodes.go file

Merge pull request #1 from appliedblockchain/sha3-256

328a9f0

Sha3 256

Merge branch 'algorand:master' into master

296528e

tsachiherman requested a review from jannotti February 1, 2022 12:55

jannotti reviewed Feb 1, 2022

View reviewed changes

Matt-Ryley added 3 commits February 15, 2022 17:36

Update eval.go

17acffe

Spelling mistake rectified

Merged upstream

5255912

Added new Opcode to assembler

8050ae4

jannotti changed the title ~~SHA3-256 added to hashing functions~~ AVM: SHA3-256 hash function opcode Mar 10, 2022

jannotti added the Enhancement label Mar 10, 2022

Matt-Ryley added 6 commits March 11, 2022 11:00

Ran make to gen spec

9d7be44

Removed darwin/arm64 FLAGS

64071ba

Didnt mean to push them!

Removed darwin/arm64 cgo

138a02a

darwin.arm64 reinstalled to batchverifier

ddf44d2

Removed binary files

54febf5

accidentally pushed all local changes!

Updated read me

5d8d05d

Matt-Ryley added 2 commits March 14, 2022 09:32

Merge upstream

732129f

Ran make in data/transaction/logic

6417786

jannotti closed this Mar 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AVM: SHA3-256 hash function opcode #3544

AVM: SHA3-256 hash function opcode #3544

Matt-Ryley commented Feb 1, 2022

CLAassistant commented Feb 1, 2022 •

edited

Loading

jannotti Feb 1, 2022

Matt-Ryley Feb 1, 2022

jannotti Feb 1, 2022

Matt-Ryley Feb 1, 2022

jannotti Feb 1, 2022 •

edited

Loading

algoanne Feb 23, 2022

Matt-Ryley Feb 24, 2022

Matt-Ryley Mar 7, 2022

Matt-Ryley Mar 7, 2022

jannotti Mar 10, 2022

jannotti commented Mar 10, 2022

codecov-commenter commented Mar 11, 2022 •

edited

Loading

Matt-Ryley commented Mar 14, 2022

jannotti commented Mar 14, 2022 via email

jannotti commented Mar 14, 2022

AVM: SHA3-256 hash function opcode #3544

AVM: SHA3-256 hash function opcode #3544

Conversation

Matt-Ryley commented Feb 1, 2022

CLAassistant commented Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannotti Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannotti commented Mar 10, 2022

codecov-commenter commented Mar 11, 2022 • edited Loading

Codecov Report

Matt-Ryley commented Mar 14, 2022

jannotti commented Mar 14, 2022 via email

jannotti commented Mar 14, 2022

CLAassistant commented Feb 1, 2022 •

edited

Loading

jannotti Feb 1, 2022 •

edited

Loading

codecov-commenter commented Mar 11, 2022 •

edited

Loading