Skip to content

feat: accept foreign tx validation requests on contract#3106

Merged
DSharifi merged 3 commits into
mainfrom
2801-arbitrum-support-in-mpc-contract
May 4, 2026
Merged

feat: accept foreign tx validation requests on contract#3106
DSharifi merged 3 commits into
mainfrom
2801-arbitrum-support-in-mpc-contract

Conversation

@DSharifi

@DSharifi DSharifi commented May 4, 2026

Copy link
Copy Markdown
Contributor

closes #2801

@DSharifi DSharifi linked an issue May 4, 2026 that may be closed by this pull request
@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Code Review

Minor issue found:

  • Mislabeled test case in crates/near-mpc-contract-interface/src/types/foreign_chain.rs: The new test case is labeled #[case::bnb(...) but it tests ForeignChainRpcRequest::Arbitrum. Should be #[case::arbitrum(...).

Otherwise the change is straightforward — adding an Arbitrum variant to ForeignChainRpcRequest with proper test coverage across sandbox tests, ABI snapshot, and unit tests. No logic, safety, or compatibility concerns.

✅ Approved (with nit above)

@pbeza pbeza requested a review from Copilot May 4, 2026 14:25
@DSharifi DSharifi enabled auto-merge May 4, 2026 14:26

@gilcu3 gilcu3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

Comment on lines 25 to 30
#[case::starknet(starknet_request(), starknet_extracted_values())]
#[case::bnb(bnb_evm_request(), evm_block_hash_extracted_values())]
#[case::base(base_evm_request(), evm_block_hash_extracted_values())]
#[case::arbitrum(arbitrum_evm_request(), evm_block_hash_extracted_values())]
#[tokio::test]
async fn verify_foreign_transaction__should_succeed(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

side comment: I wonder if we need to test each chain separately here. These tests are unfortunately getting a noticeable time inside the sandbox tests (just my experience running them locally sometimes). If the behavior in the contract is actually the same for all chains, maybe we could just test one, or use some mechanism (trait?) to ensure that testing one is enough

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't want to lose test coverage. But we could instead collapse them all to one test case, and run requests against all of them in one test similar to what @anodar did with the e2e tests

async fn verify_foreign_transaction__should_sign_all_supported_chains() {
// Given — 2-node cluster with Bitcoin, Abstract, BNB, Base, and Starknet configured
let env = setup_foreign_tx_cluster()
.await
.expect("setup_foreign_tx_cluster failed");
// When/Then — all configured chains produce valid signed responses
verify_bitcoin(&env)
.await
.expect("bitcoin verification failed");
verify_abstract(&env)
.await
.expect("abstract verification failed");
verify_bnb(&env).await.expect("bnb verification failed");
verify_base(&env).await.expect("base verification failed");
verify_starknet(&env)
.await
.expect("starknet verification failed");
verify_arbitrum(&env)
.await
.expect("arbitrum verification failed");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah that would also fix the problem indeed

@DSharifi DSharifi added this pull request to the merge queue May 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds Arbitrum support to the NEAR MPC contract “foreign tx validation request” surface by extending the contract interface DTOs and updating sandbox/ABI tests accordingly (closes #2801).

Changes:

  • Extend ForeignChainRpcRequest with a new Arbitrum(EvmRpcRequest) variant and map it to ForeignChain::Arbitrum.
  • Update the ABI snapshot to reflect the new request variant shape.
  • Add Arbitrum coverage to sandbox tests (including a new arbitrum_evm_request() helper) and to the DTO unit test cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
crates/near-mpc-contract-interface/src/types/foreign_chain.rs Adds Arbitrum to ForeignChainRpcRequest and updates chain() + tests.
crates/contract/tests/snapshots/abi__abi_has_not_changed.snap Updates ABI snapshot to include the new Arbitrum request variant schema.
crates/contract/tests/sandbox/foreign_chain_request.rs Expands sandbox foreign-tx request tests to include an Arbitrum case.
crates/contract/tests/sandbox/common.rs Adds arbitrum_evm_request() test helper for Arbitrum requests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Merged via the queue into main with commit fde3ca5 May 4, 2026
17 checks passed
@DSharifi DSharifi deleted the 2801-arbitrum-support-in-mpc-contract branch May 4, 2026 14:34
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.

Arbitrum support in mpc contract

4 participants