Skip to content

fix: add approval for subnet join#1344

Merged
phutchins merged 4 commits intomainfrom
approve
Jun 23, 2025
Merged

fix: add approval for subnet join#1344
phutchins merged 4 commits intomainfrom
approve

Conversation

@cryptoAtwill
Copy link
Copy Markdown
Contributor

Adding owner approve for subnet register. This means a subnet can be registered in the gateway only if the owner approves it.

@cryptoAtwill cryptoAtwill requested a review from a team as a code owner April 28, 2025 06:53
Comment thread contracts/contracts/gateway/GatewayManagerFacet.sol Outdated
Comment thread contracts/test/IntegrationTestBase.sol Outdated
Copy link
Copy Markdown
Contributor

@karlem karlem left a comment

Choose a reason for hiding this comment

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

Looks good. Just small details.

@karlem
Copy link
Copy Markdown
Contributor

karlem commented May 20, 2025

@cryptoAtwill I have approved but I think you need to update the e2e tests. Given the error message below it seems related to your fix.

OUT: 2025-05-20T09:27:13.660005Z ERROR ipc_provider::manager::evm::error_parsing: contract reverted with error: NotAuthorized
OUT: 2025-05-20T09:27:13.660958Z ERROR ipc_cli: main process failed: error processing command Some(Subnet(SubnetCommandsArgs { command: Join(JoinSubnetArgs { from: Some("0x9a2954b87d8745df0b1010291c51d68ae9269d43"), subnet: "/r1126193293194756/f410fhwibtof7hp6v5f453q5soyn6ksxym7chbqata2q", collateral: 1e-9, initial_balance: Some(30.0) }) })): Contract call reverted with data: 0x4a0bfec10000000000000000000000003d9019b8bf3bfd5e979ddc3b2761be54af867c47   

@cryptoAtwill
Copy link
Copy Markdown
Contributor Author

@karlem Yeah, I looked into it, I realized this is almost impossible to fix. Check out this issue: #1348. With the above issue, it means we cannot approve any subnet to be registered.

@phutchins phutchins self-requested a review June 2, 2025 16:44
@phutchins
Copy link
Copy Markdown
Contributor

Aiming to merge tomorrow.

@drahnr
Copy link
Copy Markdown
Contributor

drahnr commented Jun 20, 2025

Do we need a deployment documentation update when merging this?

@phutchins phutchins merged commit a72d2fc into main Jun 23, 2025
16 checks passed
@phutchins phutchins deleted the approve branch June 23, 2025 12:50
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.

4 participants