Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: unused vars cleanup + updated TODOs #4883

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO can't really be done because we work with fields here.

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()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTxs returns only non-empty txs so this was not necessary

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
Loading