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(multichain-testing): request body too large #9501

Merged
merged 3 commits into from
Jun 15, 2024

Conversation

0xpatrickdev
Copy link
Member

refs: #8896

Description

The current stakeIca.contract.js bundle is ~4.5 MB uncompressed. Any attempted installation resulted in 400 Bad Request: request body too large .

This change adjusts app.toml and config.toml params like max_body_bytes, max_header_bytes, max_txs_bytes, max_tx_bytes, and rpc-max-body-bytes.

Security Considerations

Scaling Considerations

The settings may not match other environments, and developers may find their bundles are too large to install if they are only developing against this environment.

Documentation Considerations

The update-config.sh is long and mostly copied from cosmology-tech/starship. I've added a note about its source and a separate commit for the adjustments made to it.

Testing Considerations

Manually tested the parameters in the course of #9042. Expect a test to be checked in with the completion of #9042.

Running the Multichain E2E Testing job and ensuring the Setup Starship Infrastructure step passes should give us confidence the overrides are also working in CI.

Upgrade Considerations

@0xpatrickdev
Copy link
Member Author

@0xpatrickdev 0xpatrickdev requested a review from turadg June 13, 2024 14:39
@@ -46,7 +46,7 @@ make stop
To setup finish setting up Agoric, also run:

```bash
make fund-provision-poool
Copy link
Member

Choose a reason for hiding this comment

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

poool was more fun

Copy link
Member Author

Choose a reason for hiding this comment

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

agree

Comment on lines 21 to 25
sed -i -e "s/max_body_bytes = .*/max_body_bytes = 15728640/g" $CHAIN_DIR/config/config.toml
sed -i -e "s/max_header_bytes = .*/max_header_bytes = 1048576/g" $CHAIN_DIR/config/config.toml
sed -i -e "s/max_txs_bytes = .*/max_txs_bytes = 1073741824/g" $CHAIN_DIR/config/config.toml
sed -i -e "s/max_tx_bytes = .*/max_tx_bytes = 15728640/g" $CHAIN_DIR/config/config.toml
sed -i -e "s/^rpc-max-body-bytes =.*/rpc-max-body-bytes = 15728640/" $CHAIN_DIR/config/app.toml
Copy link
Member

Choose a reason for hiding this comment

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

15728640 is 15 mebibytes. For whatever reason, agoric-cli's chain-config.js has 15 megabytes (15e6).

This value shouldn't be any higher. Please reference the agoric-cli value, either by importing or the comment.

sed -i -e 's/seeds = ".*"/seeds = ""/g' $CHAIN_DIR/config/config.toml
sed -i -e 's#cors_allowed_origins = \[\]#cors_allowed_origins = \["*"\]#g' $CHAIN_DIR/config/config.toml

echo "Increase `*_bytes` parameters for MsgInstallBundle"
Copy link
Member

Choose a reason for hiding this comment

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

please comment on where these values come from, with enough info that one can cross-check in case either side changes

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. It seems we only have RPC_MAX_BODY_BYTES in agoric-cli's chain-config.js, and nothing for max_header_bytes and max_txs_bytes. I transparently documented how these values were derived, and we can fix-forward if we learn there are better values to use.

Copy link

cloudflare-workers-and-pages bot commented Jun 13, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: b908567
Status: ✅  Deploy successful!
Preview URL: https://7fbe45c4.agoric-sdk.pages.dev
Branch Preview URL: https://pc-8896-starship-bundle-inst.agoric-sdk.pages.dev

View logs

@0xpatrickdev 0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Jun 14, 2024
@0xpatrickdev 0xpatrickdev force-pushed the pc/8896-starship-bundle-install branch from 4a8e045 to 2271901 Compare June 14, 2024 19:42
@0xpatrickdev 0xpatrickdev force-pushed the pc/8896-starship-bundle-install branch from 2271901 to b908567 Compare June 15, 2024 13:56
@mergify mergify bot merged commit 0850f10 into master Jun 15, 2024
63 checks passed
@mergify mergify bot deleted the pc/8896-starship-bundle-install branch June 15, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants