Skip to content

Commit

Permalink
Fix queueMessage draining all funds in sendNative on bridge (#2181)
Browse files Browse the repository at this point in the history
* Potential fix.

* Removed from other bridge.

* Added notifyDeposit function that emits an event.

* Changed the queueMessage function.

* Fixed small bug.

* A couple more bugs.

* Wiggle wiggle wiggle yeah.

---------

Co-authored-by: StefanIliev545 <[email protected]>
  • Loading branch information
StefanIliev545 and StefanIliev545 authored Dec 5, 2024
1 parent e9d7db0 commit 3ef0813
Show file tree
Hide file tree
Showing 21 changed files with 393 additions and 40 deletions.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/generated/EthereumBridge/EthereumBridge.go

Large diffs are not rendered by default.

Large diffs are not rendered by default.

170 changes: 168 additions & 2 deletions contracts/generated/MerkleTreeMessageBus/MerkleTreeMessageBus.go

Large diffs are not rendered by default.

170 changes: 168 additions & 2 deletions contracts/generated/MessageBus/MessageBus.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/generated/ObsERC20/ObsERC20.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/generated/ObscuroBridge/ObscuroBridge.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/generated/PublicCallbacks/PublicCallbacks.go

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion contracts/generated/SystemDeployer/SystemDeployer.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/generated/WrappedERC20/WrappedERC20.go

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions contracts/src/bridge/L1/ObscuroBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ contract ObscuroBridge is
data,
uint32(Topics.MANAGEMENT),
0,
0,
0
);
}
Expand All @@ -78,8 +79,6 @@ contract ObscuroBridge is
// verify ETH deposit.
function sendNative(address receiver) external payable override {
require(msg.value > 0, "Empty transfer.");
bytes memory data = abi.encode(ValueTransfer(msg.value, receiver));
queueMessage(remoteBridgeAddress, data, uint32(Topics.VALUE), 0, 0);
_messageBus().sendValueToL2{value: msg.value}(receiver, msg.value);
}

Expand Down Expand Up @@ -108,7 +107,7 @@ contract ObscuroBridge is
amount,
receiver
);
queueMessage(remoteBridgeAddress, data, uint32(Topics.TRANSFER), 0, 0);
queueMessage(remoteBridgeAddress, data, uint32(Topics.TRANSFER), 0, 0, 0);
}

function receiveAssets(
Expand Down
10 changes: 1 addition & 9 deletions contracts/src/bridge/L2/EthereumBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,6 @@ contract EthereumBridge is

function sendNative(address receiver) external payable {
require(msg.value > 0, "Nothing sent.");

bytes memory data = abi.encodeWithSelector(
IBridge.receiveAssets.selector,
localToRemoteToken[address(0x0)],
msg.value,
receiver
);
queueMessage(remoteBridgeAddress, data, uint32(Topics.TRANSFER), 0, 0);
_messageBus().sendValueToL2{value: msg.value}(receiver, msg.value);
}

Expand All @@ -85,7 +77,7 @@ contract EthereumBridge is
amount,
receiver
);
queueMessage(remoteBridgeAddress, data, uint32(Topics.TRANSFER), 0, 0);
queueMessage(remoteBridgeAddress, data, uint32(Topics.TRANSFER), 0, 0, msg.value);
}

function receiveAssets(
Expand Down
5 changes: 5 additions & 0 deletions contracts/src/messaging/IMessageBus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ interface IMessageBus {
uint64 sequence
);

event NativeDeposit(
address indexed receiver,
uint256 amount
);

// This method is called from contracts to publish messages to the other linked message bus.
// nonce - This is provided and serves as deduplication nonce. It can also be used to group a batch of messages together.
// topic - This is the topic for which the payload is published.
Expand Down
7 changes: 7 additions & 0 deletions contracts/src/messaging/MessageBus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,13 @@ contract MessageBus is IMessageBus, Initializable, OwnableUpgradeable {
);
}

function notifyDeposit(
address receiver,
uint256 amount
) external ownerOrSelf {
emit NativeDeposit(receiver, amount);
}

function retrieveAllFunds(
address receiver
) external onlyOwner {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ abstract contract CrossChainEnabledObscuro is Initializable {
bytes memory message,
uint32 topic,
uint256 gas,
uint8 consistencyLevel
uint8 consistencyLevel,
uint256 value
) internal {
bytes memory payload = abi.encode(
ICrossChainMessenger.CrossChainCall(target, message, gas)
);
messageBus.publishMessage{value: msg.value}(nonce++, topic, payload, consistencyLevel);
messageBus.publishMessage{value: value}(nonce++, topic, payload, consistencyLevel);
}
}
2 changes: 1 addition & 1 deletion contracts/src/system/PublicCallbacks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.28;
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

interface IPublicCallbacks {
function register(bytes calldata callback) external payable;
function register(bytes calldata callback) external payable returns (uint256);
function reattemptCallback(uint256 callbackId) external;
}

Expand Down
2 changes: 1 addition & 1 deletion go/enclave/components/batch_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (executor *batchExecutor) ComputeBatch(ctx context.Context, context *BatchE
messages, transfers = executor.crossChainProcessors.Local.RetrieveInboundMessages(ctx, parentBlock, block, stateDB)
}

crossChainTransactions := executor.crossChainProcessors.Local.CreateSyntheticTransactions(ctx, messages, stateDB)
crossChainTransactions := executor.crossChainProcessors.Local.CreateSyntheticTransactions(ctx, messages, transfers, stateDB)
executor.crossChainProcessors.Local.ExecuteValueTransfers(ctx, transfers, stateDB)

transactionsToProcess, freeTransactions := executor.filterTransactionsWithSufficientFunds(ctx, stateDB, context)
Expand Down
2 changes: 1 addition & 1 deletion go/enclave/crosschain/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type Manager interface {

ExtractOutboundTransfers(ctx context.Context, receipts common.L2Receipts) (common.ValueTransferEvents, error)

CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, rollupState *state.StateDB) common.L2Transactions
CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, rollupState *state.StateDB) common.L2Transactions

ExecuteValueTransfers(ctx context.Context, transfers common.ValueTransferEvents, rollupState *state.StateDB)

Expand Down
35 changes: 26 additions & 9 deletions go/enclave/crosschain/message_bus_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ func (m *MessageBusManager) ExecuteValueTransfers(ctx context.Context, transfers
}

// CreateSyntheticTransactions - generates transactions that the enclave should execute internally for the messages.
func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, rollupState *state.StateDB) common.L2Transactions {
if len(messages) == 0 {
func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, rollupState *state.StateDB) common.L2Transactions {
if len(messages) == 0 && len(transfers) == 0 {
return make(common.L2Transactions, 0)
}

Expand All @@ -223,13 +223,13 @@ func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, mes
// There can be forks thus we cannot trust the wallet.
startingNonce := rollupState.GetNonce(common.MaskedSender(*m.messageBusAddress))

signedTransactions := make(types.Transactions, 0)
syntheticTransactions := make(types.Transactions, 0)
for idx, message := range messages {
delayInBlocks := big.NewInt(int64(message.ConsistencyLevel))
data, err := MessageBusABI.Pack("storeCrossChainMessage", message, delayInBlocks)
if err != nil {
m.logger.Crit("Failed packing submitOutOfNetwork message!")
return signedTransactions
m.logger.Crit("Failed packing storeCrossChainMessage message!")
return syntheticTransactions

// todo (@stefan) - return error
// return nil, fmt.Errorf("failed packing submitOutOfNetworkMessage %w", err)
Expand All @@ -244,12 +244,29 @@ func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, mes
To: m.messageBusAddress,
}

stx, err := m.wallet.SignTransaction(tx)
stx := types.NewTx(tx)
syntheticTransactions = append(syntheticTransactions, stx)
}

startingNonce += uint64(len(messages))

for idx, transfer := range transfers {
data, err := MessageBusABI.Pack("notifyDeposit", transfer.Receiver, transfer.Amount)
if err != nil {
panic(err)
m.logger.Crit("Failed packing notifyDeposit message!")
return syntheticTransactions
}

tx := &types.LegacyTx{
Nonce: startingNonce + uint64(idx),
Value: gethcommon.Big0,
Data: data,
To: m.messageBusAddress,
Gas: 5_000_000,
GasPrice: gethcommon.Big0, // Synthetic transactions are on the house. Or the house.
}
signedTransactions = append(signedTransactions, stx)
syntheticTransactions = append(syntheticTransactions, types.NewTx(tx))
}

return signedTransactions
return syntheticTransactions
}
2 changes: 1 addition & 1 deletion integration/simulation/devnetwork/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (n *InMemNodeOperator) createEnclaveContainer(idx int) *enclavecontainer.En
GasLocalExecutionCapFlag: defaultCfg.GasLocalExecutionCapFlag,
GasPaymentAddress: defaultCfg.GasPaymentAddress,
RPCTimeout: 5 * time.Second,
SystemContractOwner: gethcommon.HexToAddress("0x0000000000000000000000000000000000000001"),
SystemContractOwner: gethcommon.HexToAddress("0xDEe530E22045939e6f6a0A593F829e35A140D3F1"),
}
return enclavecontainer.NewEnclaveContainerWithLogger(enclaveConfig, enclaveLogger)
}
Expand Down

0 comments on commit 3ef0813

Please sign in to comment.