Audit Fixes for RLP library on Broadcaster Audit

[luiz-lvj]

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

🦋 Changeset detected

Latest commit: fe917b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

Walkthrough

The RLP library in contracts/utils/RLP.sol was refactored with several changes: the encode(address) function now delegates to encode(uint256) instead of using inline assembly; readAddress removed explicit length validation and delegates to readUint256; readBytes documentation clarified about length validation handling; _decodeLength return signature changed from named variables to unnamed tuples; encode(bytes[] memory) received updated documentation about pre-encoded elements; and encode(Encoder) had formatting alignment adjustments.

Possibly related PRs

• Add RLP library #5680: Directly modifies functions and signatures in contracts/utils/RLP.sol including encode(address), readAddress, and _decodeLength.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Audit Fixes for RLP library on Broadcaster Audit' directly relates to the changeset, which contains modifications to the RLP.sol utility library addressing audit findings.
Description check	✅ Passed	The description contains a PR checklist related to standard merge requirements (Tests, Documentation, Changeset entry), which is relevant to the pull request process and changeset context.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6308fdc and 55b33a0.

📒 Files selected for processing (1)

• contracts/utils/RLP.sol (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: Redirect rules - solidity-contracts
• GitHub Check: Header rules - solidity-contracts
• GitHub Check: coverage
• GitHub Check: tests-foundry
• GitHub Check: slither
• GitHub Check: tests-upgradeable
• GitHub Check: tests
• GitHub Check: halmos
• GitHub Check: Pages changed - solidity-contracts

[Amxx]

This encoding is not consistent with how the ethereum ecosystem does things.

If you try to use ethers.js to do encode an address that had zeros, you'll get them encoded

> ethers.encodeRlp(ethers.ZeroAddress);
'0x940000000000000000000000000000000000000000'

Note that this difference is important in the context of operations like getCreateAddress. If the address prefix zeros are removed, you'd get an invalid address prediction.

I'll add unit tests to show these issues.

[ernestognw]

I mentioned to the auditors that it makes sense to include leading zeroes in addresses because there's no definition of an address in RLP. My thinking about this is that leading zeroes in an address are not necessarily "leading" because they constitute relevant information for the address.

[Amxx]

Added tests that demonstrate the breaking effect of these changes. You can see on #6107 that these tests pass on master.

---

If someone wants to encode an Address without the leading zeros, they can manually do the casting to uint256 and then call the corresponding encode function. However I believe this should not be the default encoding. In particular, predicting the addresses of contracts created using the CREATE mechanism requires the addresses to have the prefix zeros. IMO the library must offer a function that behaves that way, and I think the encode(address) is the one.

[ernestognw]

Isn't this against our guidelines?

• Numeric literals should use appropriate formats based on their purpose to improve code readability:

  Memory-related operations (use hexadecimal):

  • Memory locations: mload(64) → mload(0x40)
  • Memory offsets: mstore(add(ptr, 32), value) → mstore(add(ptr, 0x20), value)
  • Memory lengths: keccak256(ptr, 85) → keccak256(ptr, 0x55)

[ernestognw]

I don't think we need a changeset since we're not changing functionality