Skip to content

Commit

Permalink
chore: unused vars cleanup + updated TODOs (#4883)
Browse files Browse the repository at this point in the history
Just cleaning up some stale TODOs and nuking unused vars.
  • Loading branch information
benesjan committed Mar 5, 2024
1 parent 73d640a commit 3747619
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 41 deletions.
50 changes: 25 additions & 25 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ src/core/libraries/HeaderLib.sol#L150


- [ ] ID-2
[TxsDecoder.decode(bytes).vars](src/core/libraries/decoders/TxsDecoder.sol#L89) is a local variable never initialized
[TxsDecoder.decode(bytes).vars](src/core/libraries/decoders/TxsDecoder.sol#L86) is a local variable never initialized

src/core/libraries/decoders/TxsDecoder.sol#L89
src/core/libraries/decoders/TxsDecoder.sol#L86


## unused-return
Expand All @@ -50,42 +50,49 @@ src/core/Rollup.sol#L53-L91
Impact: Medium
Confidence: High
- [ ] ID-4
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L359-L361):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L360)

src/core/libraries/decoders/TxsDecoder.sol#L359-L361


- [ ] ID-5
Dubious typecast in [MessagesDecoder.read1(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L158-L160):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L159)

src/core/libraries/decoders/MessagesDecoder.sol#L158-L160


- [ ] ID-5
- [ ] ID-6
Dubious typecast in [Outbox.sendL1Messages(bytes32[])](src/core/messagebridge/Outbox.sol#L38-L46):
uint256 => uint32 casting occurs in [version = uint32(REGISTRY.getVersionFor(msg.sender))](src/core/messagebridge/Outbox.sol#L40)

src/core/messagebridge/Outbox.sol#L38-L46


- [ ] ID-6
- [ ] ID-7
Dubious typecast in [Inbox.sendL2Message(DataStructures.L2Actor,uint32,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L45-L91):
uint256 => uint64 casting occurs in [fee = uint64(msg.value)](src/core/messagebridge/Inbox.sol#L64)
uint256 => uint32 casting occurs in [entries.insert(key,fee,uint32(_recipient.version),_deadline,_errIncompatibleEntryArguments)](src/core/messagebridge/Inbox.sol#L76)

src/core/messagebridge/Inbox.sol#L45-L91


- [ ] ID-7
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L362-L364):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L363)
- [ ] ID-8
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L349-L351):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L350)

src/core/libraries/decoders/TxsDecoder.sol#L362-L364
src/core/libraries/decoders/TxsDecoder.sol#L349-L351


- [ ] ID-8
- [ ] ID-9
Dubious typecast in [MessagesDecoder.read4(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L168-L170):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L169)

src/core/libraries/decoders/MessagesDecoder.sol#L168-L170


- [ ] ID-9
- [ ] ID-10
Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L145-L189):
bytes => bytes32 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
bytes => bytes4 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
Expand Down Expand Up @@ -113,20 +120,13 @@ Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L
src/core/libraries/HeaderLib.sol#L145-L189


- [ ] ID-10
- [ ] ID-11
Dubious typecast in [Inbox.batchConsume(bytes32[],address)](src/core/messagebridge/Inbox.sol#L122-L143):
uint256 => uint32 casting occurs in [expectedVersion = uint32(REGISTRY.getVersionFor(msg.sender))](src/core/messagebridge/Inbox.sol#L128)

src/core/messagebridge/Inbox.sol#L122-L143


- [ ] ID-11
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L352-L354):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L353)

src/core/libraries/decoders/TxsDecoder.sol#L352-L354


## reentrancy-events
Impact: Low
Confidence: Medium
Expand Down Expand Up @@ -221,20 +221,20 @@ src/core/messagebridge/Inbox.sol#L21-L231
Impact: Informational
Confidence: High
- [ ] ID-22
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L294-L313) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L301-L303)

src/core/libraries/decoders/TxsDecoder.sol#L294-L313


- [ ] ID-23
[MessagesDecoder.decode(bytes)](src/core/libraries/decoders/MessagesDecoder.sol#L60-L150) uses assembly
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L79-L81)
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L112-L118)

src/core/libraries/decoders/MessagesDecoder.sol#L60-L150


- [ ] ID-23
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L291-L310) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L298-L300)

src/core/libraries/decoders/TxsDecoder.sol#L291-L310


## dead-code
Impact: Informational
Confidence: Medium
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract Rollup is IRollup {
function process(
bytes calldata _header,
bytes32 _archive,
bytes calldata _body, // TODO(#3938) Update this to pass in only th messages and not the whole body.
bytes calldata _body, // TODO(#4492) Nuke this when updating to the new message model
bytes memory _proof
) external override(IRollup) {
// Decode and validate header
Expand Down
7 changes: 2 additions & 5 deletions l1-contracts/src/core/libraries/decoders/TxsDecoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@ library TxsDecoder {
struct ArrayOffsets {
uint256 noteHash;
uint256 nullifier;
uint256 publicData;
uint256 l2ToL1Msgs;
uint256 publicData;
uint256 contracts;
uint256 contractData;
uint256 l1ToL2Msgs;
}

struct Counts {
Expand All @@ -71,11 +70,9 @@ library TxsDecoder {
// Note: Used in `computeConsumables` to get around stack too deep errors.
struct ConsumablesVars {
bytes32[] baseLeaves;
bytes32[] l2ToL1Msgs;
bytes baseLeaf;
bytes32 encryptedLogsHash;
bytes32 unencryptedLogsHash;
uint256 l1Tol2MsgsCount;
}

/**
Expand All @@ -91,8 +88,8 @@ library TxsDecoder {

{
// L1 to L2 messages
// TODO(#4492): update this when implementing the new message model
uint256 count = read4(_body, offset);
vars.l1Tol2MsgsCount = count;
offset += 0x4 + count * 0x20;

count = read4(_body, offset); // number of tx effects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct BaseOrMergeRollupPublicInputs {
// U128 isn't safe if it's an input to the circuit (it won't automatically constrain the witness)
// So we want to constrain it when casting these fields to U128

// TODO(#3938): split this to txs_hash and out_hash
// TODO(#4492): update this when implementing the new message model
// We hash public inputs to make them constant-sized (to then be unpacked on-chain)
calldata_hash : [Field; 2],
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ impl BaseRollupInputs {

let new_contracts = combined.new_contracts;
calldata_hash_inputs[offset] = new_contracts[0].contract_address.to_field();
// TODO(#3938): make portal address 20 bytes here when updating the hashing
calldata_hash_inputs[offset + 1] = new_contracts[0].portal_contract_address.to_field();

offset += MAX_NEW_CONTRACTS_PER_TX * 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl RootRollupInputs {
aggregation_object,
archive,
header,
// TODO(#3938): Nuke this once body hash/calldata hash is updated
// TODO(#4492): update this when implementing the new message model
l1_to_l2_messages_hash: compute_messages_hash(self.new_l1_to_l2_messages)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ struct RootRollupPublicInputs {
// New block header
header: Header,

// TODO(#3938): Nuke this once body hash/calldata hash is updated
// TODO(#4492): Nuke this once body hash/calldata hash is updated
l1_to_l2_messages_hash : [Field; NUM_FIELDS_PER_SHA256],
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ export class BlockStore {
});

block.getTxs().forEach((tx, i) => {
if (tx.txHash.isZero()) {
return;
}
void this.#txIndex.set(tx.txHash.toString(), [block.number, i]);
});

Expand Down
1 change: 0 additions & 1 deletion yarn-project/circuit-types/src/tx_effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export class TxEffect {
publicDataUpdateRequestsBuffer,
this.contractLeaves[0].toBuffer(),
this.contractData[0].contractAddress.toBuffer(),
// TODO(#3938): make portal address 20 bytes here when updating the hashing
this.contractData[0].portalContractAddress.toBuffer32(),
encryptedLogsHashKernel0,
unencryptedLogsHashKernel0,
Expand Down
1 change: 0 additions & 1 deletion yarn-project/foundation/src/eth-address/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ export class EthAddress {
*
* @returns A 32-byte Buffer containing the padded Ethereum address.
*/
// TODO(#3938): nuke this
public toBuffer32() {
const buffer = Buffer.alloc(32);
this.buffer.copy(buffer, 12);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ export class SoloBlockBuilder implements BlockBuilder {
// Collect all new nullifiers, commitments, and contracts from all txs in this block
const txEffects: TxEffect[] = txs.map(
tx =>
// TODO(#4720): Combined data should most likely contain the tx effect directly
new TxEffect(
tx.data.combinedData.newNoteHashes.map((c: SideEffect) => c.value) as Tuple<
Fr,
Expand Down

0 comments on commit 3747619

Please sign in to comment.