refactor: representing TxHash as Fr
#10954
Conversation
benesjan
changed the title
TxHash as Fr
| @@ -15,6 +15,6 @@ struct ProposeArgs { | |||
|
|
|||
| library ProposeLib { | |||
| function digest(ProposeArgs memory _args) internal pure returns (bytes32) { | |||
| return keccak256(abi.encode(SignatureLib.SignatureDomainSeperator.blockAttestation, _args)); | |||
| return keccak256(abi.encode(SignatureLib.SignatureDomainSeparator.blockAttestation, _args)); | |||
There was a problem hiding this comment.
This typo was in a lot of places and it bothered me. Sorry for the messy diff
| const owner = AztecAddress.fromBuffer(reader); | ||
| const contractAddress = AztecAddress.fromBuffer(reader); | ||
| const storageSlot = Fr.fromBuffer(reader); | ||
| const note = reader.readObject(Note); |
There was a problem hiding this comment.
Unnecessary change but I thought it will make it cuter
benesjan
force-pushed
the
12-23-refactor_represetnting_tx_hahs_as_fr
branch
from
a40b524 to
1c42401
Compare
| @@ -17,6 +17,7 @@ | |||
| "build": "yarn clean && tsc -b", | |||
| "start": "node --no-warnings ./dest/bin", | |||
| "start:debug": "node --inspect=0.0.0.0:9221 --no-warnings ./dest/bin", | |||
| "start:sandbox": "ETHEREUM_HOST=http://0.0.0.0:8545/ && yarn start start --sandbox", | |||
There was a problem hiding this comment.
Sneaked this in because of this discussion on slack.
benesjan
force-pushed
the
12-23-refactor_represetnting_tx_hahs_as_fr
branch
from
1c42401 to
6d9067a
Compare
| return z | ||
| .object({ | ||
| hash: schemas.Fr, | ||
| }) | ||
| .transform(({ hash }) => new TxHash(hash)); |
There was a problem hiding this comment.
Curious why this change. Before, a TxHash would be represented as just a string like 0x1234, but with this change it'd be an object like { "hash": "0x1234" }. Personally I'd prefer the more compact representation.
There was a problem hiding this comment.
I just thought that's what I need to do when doing the change to Fr (didn't really understand the schemas when first fiddling with it). I see that it was unnecessary so I reverted it in 2c021b7
Thanks for the feedback and the quick review
TxHash used to be representing as Buffer32 and it did not make sense as it's an Fr.
+ typo fixes
Note
The failing docs CI task is unrelated to this PR (Sean is just working on fixing it).