Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add v1 ArbOwnerPublic setters #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrstph-dvx
Copy link
Contributor

part of FS-529

@chrstph-dvx chrstph-dvx requested a review from spsjvc July 10, 2024 23:06
@chrstph-dvx chrstph-dvx self-assigned this Jul 10, 2024
@chrstph-dvx chrstph-dvx force-pushed the add-arbowner-setters branch from 3cdefd5 to 8c5caaa Compare July 15, 2024 14:39
import { validateChildChainPublicClient } from '../types/validateChildChainPublicClient';

export type AddChainOwnerParameters = Prettify<
WithUpgradeExecutor<
Copy link
Member

Choose a reason for hiding this comment

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

I see a type error in withUpgradeExecutor while reviewing this PR

image

...withUpgradeExecutor({
to: arbOwner.address,
upgradeExecutor,
args: [recipient],
Copy link
Member

Choose a reason for hiding this comment

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

i am wondering if we would like to check the Address type in this function, or outside? currently it is set up to be done outside of this function

what are your thoughts?

@chrstph-dvx chrstph-dvx force-pushed the add-chain-public-actions-setter branch 2 times, most recently from e8786b0 to 4ca765e Compare August 19, 2024 11:25
Base automatically changed from add-chain-public-actions-setter to main August 27, 2024 15:58
@chrstph-dvx chrstph-dvx changed the base branch from main to add-arbgasinfo-getters September 24, 2024 14:47
@chrstph-dvx chrstph-dvx changed the base branch from add-arbgasinfo-getters to main September 24, 2024 14:47
@chrstph-dvx chrstph-dvx force-pushed the add-arbowner-setters branch 3 times, most recently from 6b433ea to 2d2793b Compare September 24, 2024 15:10
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