compute the hash of the proposal iff the proposal hash is present #1365
Conversation
|
@anorth please check this thoroughly. I believe this is the correct thing to do, but I'm not familiar with why the code was written the way it was in the first place. |
Codecov Report
@@ Coverage Diff @@
## master #1365 +/- ##
======================================
Coverage 69.5% 69.5%
======================================
Files 72 72
Lines 7638 7638
======================================
Hits 5310 5310
Misses 1437 1437
Partials 891 891 |
ZenGround0
force-pushed
the
feat/optimize-msig
branch
from
0167588
to
b0bf345
Compare
|
Looks good. Adding tests before merging. |
Codecov Report
@@ Coverage Diff @@
## master #1365 +/- ##
======================================
Coverage 71.1% 71.1%
======================================
Files 72 72
Lines 8395 8395
======================================
Hits 5975 5975
Misses 1574 1574
Partials 846 846 |
|
@droark do you see any problems with this? |
ZenGround0
force-pushed
the
feat/optimize-msig
branch
from
cbac1a6
to
973af29
Compare
Previously, we'd always compute the hash but only _compare_ it if it was specified. This simply increased the cost of messages that didn't specify the hash for no good reason. fixes #1364
ZenGround0
force-pushed
the
feat/optimize-msig
branch
from
973af29
to
1b7dd18
Compare
There was a problem hiding this comment.
👍
I'd prefer that the PR not be pulled for another day, in case I think of some reason why this shouldn't be pulled, but I'm pretty sure this is safe. I can't think of any reasons why a hash should be mandatory in this situation. The modified logic seems safe too.
Previously, we'd always compute the hash but only compare it if it was specified. This simply increased the cost of messages that didn't specify the hash for no good reason.
fixes #1364