Closed edmundedgar closed 9 months ago
Very nice that you added scripts to verify it. Though it is not clear to me how to run them. Would have loved to see a testplan that shows me how to verify your claims.
Probably, there is also an error in the scripts, as I think I found logical modifications.
I really like that you discovered in solidity if() revert Error(), without {} saves quite some lines. We should remove them also in our other repo
@josojo I added a script to go back and log the earlier bytecode hashes.
Copy scripts/log_bytecode_history.sh
to some other name (so it doesn't remove itself when it checks out an earlier version) then run it with the number of commits you want to check, like 35. You may also need to edit it to update your path to solc 0.8.20. It'll work back through the history, compile each version and log the hash to a log file.
Hopefully the result will be that the only bytecode change is where we do the revert errors, which is 9450daf5769ab14a58cb365d6c8e7a78b81efc71, and the earlier refactoring of modifiers at 7f8a9b7eca1c4c7d4756769950aad3eb1e8a744c and 82b6e35bd3891af116042896981983041371498f which are intended to keep the same behaviour but will produce different bytecode.
If you see a logic change that isn't in there then the script is broken, and if you see a logic change that is in there then it's a mistake, so either way let me know where you see it.
@josojo I added a script to go back and log the earlier bytecode hashes.
Copy
scripts/log_bytecode_history.sh
to some other name (so it doesn't remove itself when it checks out an earlier version) then run it with the number of commits you want to check, like 35. You may also need to edit it to update your path to solc 0.8.20. It'll work back through the history, compile each version and log the hash to a log file.Hopefully the result will be that the only bytecode change is where we do the revert errors, which is 9450daf, and the earlier refactoring of modifiers at 7f8a9b7 and 82b6e35 which are intended to keep the same behaviour but will produce different bytecode.
If you see a logic change that isn't in there then the script is broken, and if you see a logic change that is in there then it's a mistake, so either way let me know where you see it.
With that the PR is sooo much easier to review! thanks a lot!
If I am running the code with solcjs like this:
COUNT=$1
ORIG_BRANCH=`git rev-parse --abbrev-ref HEAD`
SOLC="solcjs"
for i in $(seq $COUNT); do
sleep 1
git checkout HEAD^
SAME_BYTECODE=`git log HEAD^..HEAD --oneline | grep -c "no bytecode change"`
COMMIT=`git rev-parse HEAD`
solcjs --bin /Users/josojo/coding/forks/reality-eth-monorepo/packages/contracts/development/contracts/RealityETH_ERC20-3.0.sol --optimize
solcjs --bin /Users/josojo/coding/forks/reality-eth-monorepo/packages/contracts/development/contracts/RealityETH-3.0.sol --optimize
NEW_HASH=`sha256sum _Users_josojo_coding_forks_reality-eth-monorepo_packages_contracts_development_contracts_RealityETH_ERC20-3_0_sol_RealityETH_ERC20_v3_0.bin | head -c64`
LOG="COMMIT_LOG_ERC20_REALITY_ETH.log"
echo "${COMMIT}:${NEW_HASH}:${SAME_BYTECODE}" >> $LOG
NEW_HASH=`sha256sum _Users_josojo_coding_forks_reality-eth-monorepo_packages_contracts_development_contracts_RealityETH-3_0_sol_RealityETH_v3_0.bin | head -c64`
LOG="COMMIT_LOG_NORMAL_REALITY_ETH.log"
echo "${COMMIT}:${NEW_HASH}:${SAME_BYTECODE}" >> $LOG
done
git checkout $ORIG_BRANCH
I get for the normal realityETH:
2a4650312d16a10dd2f8a4d1f8fb207d0d35bf14:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
2a4650312d16a10dd2f8a4d1f8fb207d0d35bf14:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
34ac315394b6693a0ecbfd3284c593d097eff989:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
3760a8f9e020ba293ab9ff78e5958b690d93c9ea:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
2cc0e23291c54f1438663a03fbf0d146b059b55b:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
ebe82ceaed14ca5c31c0deb93e9db06c455f5545:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
a156982581dcf220943488103cfd9e9856847e95:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:1
61ba2405aa8e770d691805c9e405cb52aae66758:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
d3790e87e257bb99774fedcdb2598167878ed46c:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
20a0f7c3d805662c34df31186c723ed886b9421e:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
7540dc473e5ab7a8ad003a873632c6abb53c9395:d86641acd5654ed121559c1bd29c65ee13ae9bd4f1239a740cf415825b2266cc:0
89cf75b2d617b903b799991df2d9ee7c2901596c:41a22d24cefbab693b6133b84f696b023335d93afd9c775d18259204a38cf7f7:0
674f141b4ed2b4501531ab0f29be9f9bf3168191:41a22d24cefbab693b6133b84f696b023335d93afd9c775d18259204a38cf7f7:0
bda6ce57b64b56c603d1ea822339ef475a5b8b57:41a22d24cefbab693b6133b84f696b023335d93afd9c775d18259204a38cf7f7:1
2e7bbf335dfffb440c13a08f5d55f710223f2278:41a22d24cefbab693b6133b84f696b023335d93afd9c775d18259204a38cf7f7:1
524c0066459f82dea90821ab60a50bd70a0bc0d4:f88eb7744fe65f35da22419fdd545d234a946638b49faaf6d4599b15393f8b92:0
dc4a9d981e6838a768a1411d85a75444022b03b9:f88eb7744fe65f35da22419fdd545d234a946638b49faaf6d4599b15393f8b92:0
a55c42b48eba3e2261edbd2a72ce2323c7068269:f88eb7744fe65f35da22419fdd545d234a946638b49faaf6d4599b15393f8b92:0
4efbae0910667010ede5847855aebddf9947c9f4:f88eb7744fe65f35da22419fdd545d234a946638b49faaf6d4599b15393f8b92:1
d0a41460628bb846ee975968cd03ca8694c4f003:ac7b70ce7efa37cd3ac02475b51ef6922c67ee3c0e0546e88df18bed29800432:0
86288e3ea5c6801248bc522fbdfc8e7dba091226:ac7b70ce7efa37cd3ac02475b51ef6922c67ee3c0e0546e88df18bed29800432:0
8ef352efae68fe29e65a3386e8ce9d34364c0971:ac0e1a79ba5c26fef90d7d996ed7c4630410ec2b13d8afafea7861d82e47a751:0
012f794e40fd08487ebf081b99fef3981674a1ff:ac0e1a79ba5c26fef90d7d996ed7c4630410ec2b13d8afafea7861d82e47a751:0
9450daf5769ab14a58cb365d6c8e7a78b81efc71:ac0e1a79ba5c26fef90d7d996ed7c4630410ec2b13d8afafea7861d82e47a751:0
4e8c9436b6fcb6e93451cb77e7f08a61b696722f:34dd595cb5f8180442b928fb10e69e47a7e015de1d22f437f12bccf2a5f6fb41:0
48e7dc0a1a4868f4bb3f9aebd1768fd0e93cef80:34dd595cb5f8180442b928fb10e69e47a7e015de1d22f437f12bccf2a5f6fb41:1
d2d95796c438a0227057049dce5beeacd4e2d8b9:84b095bbe75688919c0f36318bc8a62d09063aadd4a9399b5adc43b49227ee1b:1
c08ac7ac6f545f316c6822f5420756cded9d0e29:c9e80140b597fad39829b9b326291883b909fee03467313c82933601e3e3b3ac:1
75c927296c03631126062d19408efb098eae23c7:1a6921d0d2c5e1cea768bc4e19758d6de54c2a444179e94f6a0f89eb7593139d:1
00610c4c5b924f238458519f267bcdbed04010f5:9ac493321a5ff92e8a234db699f0e4366326c1b9a90a340865acebe125e6facf:0
6109225ff0b1bb36e1bbc3327e566a4a2c880818:9ac493321a5ff92e8a234db699f0e4366326c1b9a90a340865acebe125e6facf:1
47d75727aa835887cd18cf371d53723f8e2f70bb:ae4cbfd2fa795ae44766f42a7056c91f8f11c9e920786b0e9577a7567f2adeb0:1
0643ac42fd6481fc67ce07ad3fde902420de4c9d:cdc20c1743a0acb48fb79c7c3bc34ce1f00acb37dfafcc962f6f9b32721b6ee5:0
c906cba153d03c315cc77e1bc99386c829196463:cdc20c1743a0acb48fb79c7c3bc34ce1f00acb37dfafcc962f6f9b32721b6ee5:0
fdf1f6c53aa655a6b9c268616727220d32ff1c40:f3ca5be67458139c47776b3a10c711422291a7fa6b84180b5fb8f67dce3abe55:0
82b6e35bd3891af116042896981983041371498f:e59fe91ef080ac7fff7db728cc3b4f892b13578e53178c8e95c31031a9a5cd24:0
3789ff94b0e8d704d374dc8a59a3c2eac138af0c:f026c0c90b4758b8086ba5a4968d36ceb0a3ddbd510539359587c4d447b57c18:0
7f8a9b7eca1c4c7d4756769950aad3eb1e8a744c:f026c0c90b4758b8086ba5a4968d36ceb0a3ddbd510539359587c4d447b57c18:0
bcceffa92e6f6b55ae4c3625a2d62a72e17186bc:4223a097589e2a67517c7a92a8a2f8b1b452cf47f7fecfb5d35596cfd14ce122:0
026f97791f5358d4aea8ee6affb8bb7a7e5546c4:4223a097589e2a67517c7a92a8a2f8b1b452cf47f7fecfb5d35596cfd14ce122:0
a98320cc82c1baf1aa61ca18e2e0d26a3f6b5b77:4223a097589e2a67517c7a92a8a2f8b1b452cf47f7fecfb5d35596cfd14ce122:0
Not sure why it works for you so much better. Maybe the optimizer is not deterministic. (if I would run it without optimize, it would say that contracts are too big) solc install would somehow always fail on my computer when installing it with brew.
PR looks really good. Very nice that you provided the tools to check for bytecode changes. Love it.
(If you can provide a clean log from your computer that shows that I am just having weird issues, then I am happy to approve)
This should make no changes to functionality except for using revert errors instead of revert strings.