[blueogin]

Description

Receive side. A limit-blocked receive was handled two different ways depending on whether the payload happened to carry a compose message: a plain transfer was parked as a failed request, while a composed one reverted. The reason was real — a parked request could only _credit, so parking a composed message would silently drop its compose payload — but the result was that payload length decided the recovery semantics.

Send side. _debit pulled the user's tokens into the adapter, approved the MinterBurner, and let it pull them a second time. That meant two token transfers plus an allowance per send, the adapter briefly custodied user funds, and TokensBurned named the adapter as the burn source, so nothing on-chain tied a burn to the account that funded it

About # (link your issue here)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• PR title matches follow: (Feature|Bug|Chore) Task Name
• My code follows the style guidelines of this project
• I have followed all the instructions described in the initial task (check Definitions of Done)
• I have performed a self-review of my own code
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have added reference to a related issue in the repository
• I have added a detailed description of the changes proposed in the pull request. I am as descriptive as possible, assisting reviewers as much as possible.
• I have added screenshots related to my pull request (for frontend tasks)
• I have pasted a gif showing the feature.
• @mentions of the person or team responsible for reviewing proposed changes

Summary by Sourcery

Unify failed OFT receive recovery and streamline outbound token burning while preserving sender attribution.

Bug Fixes:

• Ensure limit-blocked OFT receives use a single recovery path that preserves and replays composed payloads.
• Prevent duplicate token transfers during sends and attribute burned funding to the original sender.

Enhancements:

• Store the complete message and replay origin for failed receive recovery while deriving recipient and amount on demand.
• Add failed-request details decoding and sender burn attribution events.
• Restrict MinterBurner burns to tokens already held by the burner contract.

Deployment:

• Document the incompatible failed-request storage layout and the requirement to drain parked requests before upgrading deployed instances.

Tests:

• Expand receive and burn tests to cover composed-message replay, recovery details, access control, reentrancy, and direct-to-burner token flows.

[sourcery-ai]

Hey - I've found 5 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol" line_range="107" />
<code_context>
         uint32 srcEid;
+        uint64 nonce;
+        bytes32 sender;
+        bytes message;
     }
-    /// @dev A mapping for failed requests
</code_context>
<issue_to_address>
**Pending requests become unrecoverable**

When the adapter is upgraded while failed requests stored under the old layout remain pending, `approveFailedRequest` reads the old mapping slots as the new replay fields, so it replays corrupted data and reverts; the original receive remains uncredited. The source-side burn has no matching destination credit.

Migrate or drain old-layout requests before upgrading, or preserve their storage layout and replay compatibility.
</issue_to_address>

### Comment 2
<location path="packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol" line_range="334" />
<code_context>
+        require(msg.sender == address(this), 'only self');
+        // _message[:0] is an empty calldata slice: OFTCore._lzReceive ignores _executor and _extraData,
+        // and a literal cannot be passed where calldata is expected
+        super._lzReceive(_origin, _guid, _message, address(0), _message[:0]);
     }

</code_context>
<issue_to_address>
**Approval bypasses the whitelist**

When a composed request is parked while whitelisting is enabled and its recipient is removed before approval, `approveFailedRequest` checks only `isClosed`, then `replayFailedRequest` calls `super._lzReceive` without rechecking the whitelist; the recipient is credited despite the whitelist policy.

Recheck the recipient's whitelist status when approving or replaying a failed request.

Also at `packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:510`.
</issue_to_address>

### Comment 3
<location path="packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol" line_range="576" />
<code_context>
+            innerToken.balanceOf(address(minterBurner)) - balanceBefore == amountSentLD,
+            'received amount mismatch'
+        );
+        require(minterBurner.burn(address(minterBurner), amountSentLD), 'GoodDollarOFTAdapter: Burn failed');
+        emit SenderTokensBurned(_from, amountSentLD);
     }
</code_context>
<issue_to_address>
**Mixed upgrades halt outbound sends**

When the adapter and MinterBurner proxies run different implementations during separate upgrades, the new `_debit` and `burn` calling convention is incompatible with either old implementation, so outbound sends revert until both proxies are upgraded.

Make the burn flow compatible across implementation versions or upgrade both proxies atomically before accepting sends.

Also at `packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol:74`.
</issue_to_address>

### Comment 4
<location path="packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol" line_range="107" />
<code_context>
         uint32 srcEid;
+        uint64 nonce;
+        bytes32 sender;
+        bytes message;
     }
-    /// @dev A mapping for failed requests
</code_context>
<issue_to_address>
**Failed-message tests cannot read payload**

When the new unit tests read a parked request through `failedReceiveRequests(guid)`, solidity omits dynamically sized struct members from the autogenerated getter for `failedReceiveRequests`, so the test's `req.message` is undefined and its equality assertions fail. The adapter has no explicit getter for the parked message.

Add an explicit view function that returns the stored message and use it in these assertions, or remove the inaccessible-field assertions.
</issue_to_address>

### Comment 5
<location path="packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol" line_range="105-107" />
<code_context>
         uint64 timestamp;
-        uint256 amount;
         uint32 srcEid;
+        uint64 nonce;
+        bytes32 sender;
+        bytes message;
     }
-    /// @dev A mapping for failed requests
</code_context>
<issue_to_address>
**Legacy request reads return wrong fields**

When an off-chain consumer continues using the pre-upgrade ABI for the public mapping getter, legacy RPC clients decode `failedReceiveRequests(bytes32)` using the old tuple (`failed`, `toAddress`, `timestamp`, `amount`, `srcEid`), but the public getter now returns fields from the new struct layout. The call still decodes, but clients show incorrect request metadata and can make recovery decisions from it.

Preserve the old getter’s return shape or provide a clearly versioned compatibility API and update consumers to the new ABI.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 5 findings to address first, and the changed mapping value layout makes previously parked requests unreadable to this implementation, so their already-consumed LayerZero messages cannot be approved and the intended token credit may remain stuck; the diff says those records must be drained before upgrading a deployment that has them. The bridge replay and burn paths also affect token issuance and destruction, which cannot be undone merely by reverting after a credit or burn has executed.

Blocking findings: packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:107, packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:334, packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:576, packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:107, packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol:107

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

[sourcery-ai]

🟡 Medium · Mixed upgrades halt outbound sends

When the adapter and MinterBurner proxies run different implementations during separate upgrades, the new _debit and burn calling convention is incompatible with either old implementation, so outbound sends revert until both proxies are upgraded.

Make the burn flow compatible across implementation versions or upgrade both proxies atomically before accepting sends.

Also at packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol:74.

Prompt for AI agents

In `packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol` at line 576:

**Mixed upgrades halt outbound sends**

When the adapter and MinterBurner proxies run different implementations during separate upgrades, the new `_debit` and `burn` calling convention is incompatible with either old implementation, so outbound sends revert until both proxies are upgraded.

Make the burn flow compatible across implementation versions or upgrade both proxies atomically before accepting sends.

Also at `packages/bridge-contracts/contracts/oft/GoodDollarOFTMinterBurner.sol:74`.