Added vaultSharesToken to IHyperdrive.PoolConfig

[jalextowle]

Description

This PR adds a new field to IHyperdrive.PoolConfig called vaultSharesToken. This unifies the interface for getting the vault shares token (something that was previously fragmented across different getters per instance) and also unifies the immutables used by different instances to reduce the complexity of instance implementations.

Review Checklists

Please check each item before approving the pull request. While going
through the checklist, it is recommended to leave comments on items that are
referenced in the checklist to make sure that they are reviewed. If there are
multiple reviewers, copy the checklists into sections titled ## [Reviewer Name].
If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can
be removed.

[[Reviewer Name]]

Solidity

• Tokens
  • Do all approve calls use forceApprove?
  • Do all transfer calls use safeTransfer?
  • Do all transferFrom calls use msg.sender as the from address?
    • If not, is the function access restricted to prevent unauthorized
      token spend?
• Low-level calls (call, delegatecall, staticcall, transfer, send)
  • Is the returned success boolean checked to handle failed calls?
  • If using delegatecall, are there strict access controls on the
    addresses that can be called? It shouldn't be possible to delegatecall
    arbitrary addresses, so the list of possible targets should either be
    immutable or tightly controlled by an admin.
• Reentrancy
  • Are functions that make external calls or transfer ether marked as nonReentrant?
    • If not, is there documentation that explains why reentrancy is
      not a concern or how it's mitigated?
• Gas Optimizations
  • Is the logic as simple as possible?
  • Are the storage values that are used repeatedly cached in stack or
    memory variables?
  • If loops are used, are there guards in place to avoid out-of-gas
    issues?
• Visibility
  • Are all payable functions restricted to avoid stuck ether?
• Math
  • Is all of the arithmetic checked or guarded by if-statements that will
    catch underflows?
  • If Safe functions are altered, are potential underflows and
    overflows caught so that a failure flag can be thrown?
  • Are all of the rounding directions clearly documented?
• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?

Rust

• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?
  • If matching Solidity behavior, are there differential fuzz tests that
    ensure that Rust matches Solidity?

[[Matt Brown]]

Solidity

• Tokens
  • Do all approve calls use forceApprove?
  • Do all transfer calls use safeTransfer?
  • Do all transferFrom calls use msg.sender as the from address?
    • If not, is the function access restricted to prevent unauthorized
      token spend?
• Low-level calls (call, delegatecall, staticcall, transfer, send)
  • Is the returned success boolean checked to handle failed calls?
  • If using delegatecall, are there strict access controls on the
    addresses that can be called? It shouldn't be possible to delegatecall
    arbitrary addresses, so the list of possible targets should either be
    immutable or tightly controlled by an admin.
• Reentrancy
  • Are functions that make external calls or transfer ether marked as nonReentrant?
    • If not, is there documentation that explains why reentrancy is
      not a concern or how it's mitigated?
• Gas Optimizations
  • Is the logic as simple as possible?
  • Are the storage values that are used repeatedly cached in stack or
    memory variables?
  • If loops are used, are there guards in place to avoid out-of-gas
    issues?
• Visibility
  • Are all payable functions restricted to avoid stuck ether?
• Math
  • Is all of the arithmetic checked or guarded by if-statements that will
    catch underflows?
  • If Safe functions are altered, are potential underflows and
    overflows caught so that a failure flag can be thrown?
  • Are all of the rounding directions clearly documented?
• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?

Rust

• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?
  • If matching Solidity behavior, are there differential fuzz tests that
    ensure that Rust matches Solidity?

@mcclurejt

Solidity

• Tokens
  • Do all approve calls use forceApprove?
  • Do all transfer calls use safeTransfer?
  • Do all transferFrom calls use msg.sender as the from address?
    • If not, is the function access restricted to prevent unauthorized
      token spend?
• Low-level calls (call, delegatecall, staticcall, transfer, send)
  • Is the returned success boolean checked to handle failed calls?
  • If using delegatecall, are there strict access controls on the
    addresses that can be called? It shouldn't be possible to delegatecall
    arbitrary addresses, so the list of possible targets should either be
    immutable or tightly controlled by an admin.
• Reentrancy
  • Are functions that make external calls or transfer ether marked as nonReentrant?
    • If not, is there documentation that explains why reentrancy is
      not a concern or how it's mitigated?
• Gas Optimizations
  • Is the logic as simple as possible?
  • Are the storage values that are used repeatedly cached in stack or
    memory variables?
  • If loops are used, are there guards in place to avoid out-of-gas
    issues?
• Visibility
  • Are all payable functions restricted to avoid stuck ether?
• Math
  • Is all of the arithmetic checked or guarded by if-statements that will
    catch underflows?
  • If Safe functions are altered, are potential underflows and
    overflows caught so that a failure flag can be thrown?
  • Are all of the rounding directions clearly documented?
• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?

Rust

• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?
  • If matching Solidity behavior, are there differential fuzz tests that
    ensure that Rust matches Solidity?

Cash

Solidity

• Tokens
  • Do all approve calls use forceApprove?
  • Do all transfer calls use safeTransfer?
  • Do all transferFrom calls use msg.sender as the from address?
    • If not, is the function access restricted to prevent unauthorized
      token spend?
• Low-level calls (call, delegatecall, staticcall, transfer, send)
  • Is the returned success boolean checked to handle failed calls?
  • If using delegatecall, are there strict access controls on the
    addresses that can be called? It shouldn't be possible to delegatecall
    arbitrary addresses, so the list of possible targets should either be
    immutable or tightly controlled by an admin.
• Reentrancy
  • Are functions that make external calls or transfer ether marked as nonReentrant?
    • If not, is there documentation that explains why reentrancy is
      not a concern or how it's mitigated?
• Gas Optimizations
  • Is the logic as simple as possible?
  • Are the storage values that are used repeatedly cached in stack or
    memory variables?
  • If loops are used, are there guards in place to avoid out-of-gas
    issues?
• Visibility
  • Are all payable functions restricted to avoid stuck ether?
• Math
  • Is all of the arithmetic checked or guarded by if-statements that will
    catch underflows?
  • If Safe functions are altered, are potential underflows and
    overflows caught so that a failure flag can be thrown?
  • Are all of the rounding directions clearly documented?
• Testing
  • Are there new or updated unit or integration tests?
  • Do the tests cover the happy paths?
  • Do the tests cover the unhappy paths?
  • Are there an adequate number of fuzz tests to ensure that we are
    covering the full input space?

[github-actions]

Hyperdrive Gas Benchmark

Benchmark suite	Current: c79b0f1	Previous: 46c834d	Deviation	Status
addLiquidity: min	33915 gas	1546 gas	2093.7257%	🚨
addLiquidity: avg	145391 gas	67936 gas	114.0117%	🚨
addLiquidity: max	405253 gas	293159 gas	38.2366%	🚨
checkpoint: min	29232 gas	1182 gas	2373.0964%	🚨
checkpoint: avg	104013 gas	48477 gas	114.5615%	🚨
checkpoint: max	211255 gas	191559 gas	10.2819%	🚨
closeLong: min	31428 gas	1492 gas	2006.4343%	🚨
closeLong: avg	132882 gas	29374 gas	352.3797%	🚨
closeLong: max	295147 gas	152303 gas	93.7894%	🚨
closeShort: min	31349 gas	1494 gas	1998.3266%	🚨
closeShort: avg	127732 gas	33444 gas	281.9280%	🚨
closeShort: max	201614 gas	149156 gas	35.1699%	🚨
initialize: min	31283 gas	1451 gas	2055.9614%	🚨
initialize: avg	252984 gas	213962 gas	18.2378%	🚨
initialize: max	322173 gas	253953 gas	26.8632%	🚨
openLong: min	33503 gas	1499 gas	2135.0233%	🚨
openLong: avg	166815 gas	51658 gas	222.9219%	🚨
openLong: max	252525 gas	185562 gas	36.0866%	🚨
openShort: min	33848 gas	1520 gas	2126.8421%	🚨
openShort: avg	169736 gas	51482 gas	229.6997%	🚨
openShort: max	363609 gas	181325 gas	100.5289%	🚨
redeemWithdrawalShares: min	31227 gas	1488 gas	1998.5887%	🚨
redeemWithdrawalShares: avg	63364 gas	22375 gas	183.1911%	🚨
redeemWithdrawalShares: max	161900 gas	109301 gas	48.1231%	🚨
removeLiquidity: min	31323 gas	1530 gas	1947.2549%	🚨
removeLiquidity: avg	218865 gas	151225 gas	44.7281%	🚨
removeLiquidity: max	376714 gas	325781 gas	15.6341%	🚨

This comment was automatically generated by workflow using github-action-benchmark.

[sentilesdal]

Went through the checklist, looks mostly mechanincal though. I double-checked that the integration tokens don't have safeTranfserFrom and left comments. We might want to add a comment to those tokens to make future reviews easier.

After this lands I'll go through the codegen again and update to these changes.

[mcclurejt]

LGTM, make sure to address FIXME if needed

[jrhea]

im going to go ahead and approve, but there are some nits to address. The test coverage has dipped. Can we add this to our priorities? The raw number is a bit of a vanity metric, but the change in coverage is relevant