Skip to content

[Bug] miner_setMaxDASize should return bool type#346

Merged
mattsse merged 1 commit intoalloy-rs:mainfrom
ffddw:issues/fix-return-type
Dec 17, 2024
Merged

[Bug] miner_setMaxDASize should return bool type#346
mattsse merged 1 commit intoalloy-rs:mainfrom
ffddw:issues/fix-return-type

Conversation

@SangyunOck
Copy link
Copy Markdown

@SangyunOck SangyunOck commented Dec 17, 2024

Motivation

op batcher prints error
ERROR[12-17|18:17:23.568] Result of SetMaxDASize was false, retrying.

command for op-bathcer

op-batcher \
  --l2-eth-rpc=http://localhost:8545 \
  --rollup-rpc=http://localhost:9545 \
  --poll-interval=1s \
  --sub-safety-margin=6 \
  --num-confirmations=1 \
  --safe-abort-nonce-too-low-count=3 \
  --resubmission-timeout=30s \
  --rpc.addr=0.0.0.0 \
  --rpc.port=8548 \
  --rpc.enable-admin \
  --max-channel-duration=25 \
  --l1-eth-rpc=$L1_RPC_URL \
  --private-key=$GS_BATCHER_PRIVATE_KEY

golang implementation of optimism's op-geth returns bool type for miner_setMaxDASize call. Therefore, expected response might be like following:

{
  "jsonrpc": "2.0",
  "id": 0,
  "result": true
}

On the other hand, current trait returns (), represents null for json.

pub trait MinerApiExt {
    /// Sets the maximum data availability size of any tx allowed in a block, and the total max l1
    /// data size of the block. 0 means no maximum.
    #[method(name = "setMaxDASize")]
    async fn set_max_da_size(&self, max_tx_size: U64, max_block_size: U64) -> RpcResult<()>; // should return bool
}
{
  "jsonrpc": "2.0",
  "id": 0,
  "result": null
}

ref: #325, #345

Solution

alter () to bool for return type

async fn set_max_da_size(&self, max_tx_size: U64, max_block_size: U64) -> RpcResult<bool>;

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

-.-

@mattsse mattsse added this pull request to the merge queue Dec 17, 2024
Merged via the queue into alloy-rs:main with commit 3584cc6 Dec 17, 2024
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 21, 2026
<!--
Thank you for your Pull Request. Please provide a description above and
review
the requirements below.

Bug fixes and new features should include tests.

Contributors guide:
https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md

The contributors guide includes instructions for running rustfmt and
building the
documentation.
-->

<!-- ** Please select "Allow edits from maintainers" in the PR Options
** -->

## Motivation

op batcher prints error
`ERROR[12-17|18:17:23.568] Result of SetMaxDASize was false, retrying.`

command for op-bathcer
```shell
op-batcher \
  --l2-eth-rpc=http://localhost:8545 \
  --rollup-rpc=http://localhost:9545 \
  --poll-interval=1s \
  --sub-safety-margin=6 \
  --num-confirmations=1 \
  --safe-abort-nonce-too-low-count=3 \
  --resubmission-timeout=30s \
  --rpc.addr=0.0.0.0 \
  --rpc.port=8548 \
  --rpc.enable-admin \
  --max-channel-duration=25 \
  --l1-eth-rpc=$L1_RPC_URL \
  --private-key=$GS_BATCHER_PRIVATE_KEY
```

golang implementation of optimism's op-geth [returns
bool](https://github.com/ethereum-optimism/op-geth/blob/optimism/eth/api_miner.go#L62)
type for `miner_setMaxDASize` call. Therefore, expected response might
be like following:

```json
{
  "jsonrpc": "2.0",
  "id": 0,
  "result": true
}
```

On the other hand, current trait [returns
`()`](https://github.com/alloy-rs/op-alloy/blob/main/crates/rpc-jsonrpsee/src/traits.rs#L146),
represents null for json.

```rust
pub trait MinerApiExt {
    /// Sets the maximum data availability size of any tx allowed in a block, and the total max l1
    /// data size of the block. 0 means no maximum.
    #[method(name = "setMaxDASize")]
    async fn set_max_da_size(&self, max_tx_size: U64, max_block_size: U64) -> RpcResult<()>; // should return bool
}
```

```json
{
  "jsonrpc": "2.0",
  "id": 0,
  "result": null
}
```

ref: #325, #345

## Solution

alter `()` to `bool` for return type

```rust
async fn set_max_da_size(&self, max_tx_size: U64, max_block_size: U64) -> RpcResult<bool>;
```

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [x] Breaking changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants