Fixup in parseSignature for functions with no args #72
Conversation
|
Hi @JAG-UK, thanks for the PR! 😄 Can you re-format your initial comment a bit, this is really really hard to read. Also, can you add a simple test case to the |
For sure. It looked much clearer when I originally wrote it on a monospace console with more whitespace...it is pretty grotty now, I agree. I'll remove all the testing evidence actually, since adding a test will make that unnecessary - the CI can speak for itself. |
|
There is currently still a Can you please fix and optimally squash the commit afterwards and also squash 2b89c8a to end up with a cleaner commit history (if you haven't done before I would recommend testing this with local copy of your working directory, since things can easily go wrong)? |
test/index.js
Outdated
| @@ -787,3 +787,17 @@ describe('decoding (uint[][3], uint)', function () { | |||
| assert.equal(a[0][2][1].toString(10), 6) | |||
| }) | |||
| }) | |||
|
|
|||
| //Homebrew tests for simpleEncode() family | |||
There was a problem hiding this comment.
Thanks for adding the test case, great! 👍
(side question: what is a "homebrew test"?? 😄)
There was a problem hiding this comment.
what is a "homebrew test"
One we made at home, in contrast with standard or other verified external test vectors such as the official Ethereum ones used in the very first tests. There are various words for this but Alex Beregszaszi (if git blame be believed) already chose 'Homebrew' earlier in the file so I stuck with the convention :-)
Sorry, that's me and my tight comment style again :-/ Fixing now.
Sure
I've done it before, and I've done it wrong before ;-) On it. |
Previously passing “functionWithNoArgs()” for binary encoding would fail with errors. Now it works and produces correct function hashes that match the exported functions from solidity contracts. Includes a slightly naive 'special case' fixup prior to return - not the most beautiful thing but it makes the fix very self-contained and low-risk
|
All done now I think. lints and tests clean with just 2 commits: one for the fix (including all format compliance squashed); and one for the test similarly squashed |
|
Thanks @holgerd77 , for a very pleasant and professional PR experience :-) |
Previously passing “functionWithNoArgs()” for binary encoding would fail. Now it works and produces correct function hashes that match the exported functions from solidity contracts.
Two things required to make this work:
Testing evidence: *** In a plain node console:
var abi = require('ethereumjs-abi') undefined //First test we didn’t break the existing functionality! abi.simpleEncode("inc(uint)", "0x13").toString("hex") ‘812600df0000000000000000000000000000000000000000000000000000000000000013' //Now test the new one abi.simpleEncode("meaningOfLife()").toString("hex") '5353455a' .exit
*** Then with a running network and a contract deployed with a 'meaningOfLife()' function exported: root@77d32569caa0:/project/sawtooth-seth/contracts/examples/simple_intkey# seth contract call --wait jag_x9 46ae4ccf394274f606a3e0bb397ee9fb3ad90e06 5353455a Enter Password to unlock jag_x9: Contract called Transaction Receipt: { "TransactionID": "ea5e92b0eab77bae858a632e0606989a600fe6563f2aceff9d6634b47137e0a169e10400915d3be1e4e2d8de0770f1f32dc916ee4296693e6ecb86c1193a0387", "GasUsed": 86, "ReturnValue": "000000000000000000000000000000000000000000000000000000000000002a" }