Closed nichonien closed 2 years ago
@mirceanis Did you get a chance to look the at PR?
Excellent work.
It took me a while to test this out since the existing test harness doesn't work on my machine any more. We need to upgrade the test automation here as well if we want to keep improving the contract(s).
I made some suggestions regarding the way the message is hashed in the
change/add/remove*Signed()
methods, for maintaining compatibility with existing codebases.Using
abi.encodePacked
instead ofabi.encode
allows the caller to use simple concatenation of byte arrays instead of doing RLP encoding of arguments. Also, since there is at most one variable-length array parameter, the warning in the solidity docs does not apply.
Thanks @mirceanis for the feedback. I have made the suggested changes.
I do agree that the tests need to upgraded. It broke for me as well and I could only test it with our test-suite. I guess I could give sometime post next week to improve test cases.
BTW, I have started a governance topic about did:ethr upgrades on our discord: https://discord.gg/MTeTAwSYe7
Feel free to join and share your thoughts.
:tada: This PR is included in version 1.0.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
This PR contains changes :
now
withblock.timestamp
byte
withbytes1
keccak256
implementation to one parameter implementation (latest version)0x0
address to full length address