Fix unenforced compliance/supply config and de-duplicate multisig approval logic - #468
Merged
abayomicornelius merged 4 commits intoAug 24, 2026
Conversation
…only set_carbon_credit_price/carbon_credit_price let the carbon oracle set and read "the price per carbon credit," and the value is surfaced in export_regulatory_data's report. But calculate_carbon_credits — the only function that actually computes a credits amount — never references it; credit amounts are derived purely from project.green_impact. As implemented this is a fully-functional admin setter wired to nothing, which could mislead a reader (or regulatory-report consumer) into assuming it affects credit issuance or has a monetary conversion role. Document set_carbon_credit_price as informational/reserved-for-future-use rather than silently leaving the disconnect unexplained, per the issue's own accepted scope (wire it in, or document it as informational). Closes Heliobond#456
… calls set_max_transaction_amount/max_transaction_amount let the owner configure "the maximum transaction amount limit for compliance monitoring" and the value is surfaced in export_regulatory_data's report, but it was never read anywhere else — deposit, withdraw, fund_project, and claim_insurance all moved USDC of any size regardless of what was configured. A compliance officer relying on this field to mean "transactions above this were blocked" would have been misled, since the contract silently allowed oversized transactions the whole time. Add check_max_transaction_amount (0 = no limit, matching the field's documented convention) and call it from deposit, withdraw (against the actual USDC returned), fund_project_internal, and claim_insurance_internal. New VaultError::ExceedsMaxTransactionAmount error variant. Closes Heliobond#457
…-loan mints MAX_HBS_SUPPLY (Heliobond#20) is documented as ruling out "theoretical infinite minting" and providing a predictable upper bound on total HBS supply, but only the deposit path enforced it. bridge_mint, complete_bridge_transfer (Wormhole), and execute_flash_loan each called Base::mint with no reference to the cap at all — for the bridge paths in particular this permanently increases total_supply, not just transient in-transaction state like the flash-loan case, so three of the four ways new shares can be created had no supply ceiling whatsoever. Add the same `total_shares + minted_amount > MAX_HBS_SUPPLY` check (already used in deposit) to all three mint sites before minting. Closes Heliobond#458
investment_vault and project_registry each defined their own private validate_multisig_config, require_admin_approval, and require_multisig_disabled functions. The logic was identical line-for-line (duplicate-signer detection via nested loops, threshold-vs-signer-count validation, per-approver auth + duplicate-approval checks) — the only difference was which contract's VaultError/RegistryError variant got passed to panic_with_error!. A future fix to the approval logic (e.g. a bug in duplicate-approver detection) would have needed to be applied twice and could drift, the same risk Heliobond#331 already called out for a smaller instance of copy-paste logic. Add a new no_std workspace crate, libs/multisig, holding the actual validation/approval algorithm and returning typed Result errors instead of panicking directly. Both contracts now call the shared implementation through a thin wrapper that maps the returned error onto their own contracterror enum, so on-chain error codes are unchanged for both. Closes Heliobond#459
|
@circleboyslimited Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four independent fixes in
investment_vault/project_registry, each addressing its own issue.carbon_credit_priceis configured/reported but never used (#456)set_carbon_credit_price/carbon_credit_pricelet the oracle set "the price per carbon credit," surfaced viaexport_regulatory_data, butcalculate_carbon_credits(the only function that actually computes a credits amount) never references it — credits are derived purely fromproject.green_impact. Documentedset_carbon_credit_priceas informational/reserved-for-future-use, per the issue's own accepted scope, rather than leaving the disconnect unexplained.max_transaction_amountcompliance limit never enforced (#457)Same pattern: a configured limit, surfaced in the regulatory report, that no value-moving function ever checked. Added
check_max_transaction_amount(0 = no limit, matching the field's documented convention) and wired it intodeposit,withdraw(against the actual USDC returned),fund_project_internal, andclaim_insurance_internal. NewVaultError::ExceedsMaxTransactionAmount.MAX_HBS_SUPPLYcap only checked indeposit()(#458)bridge_mint,complete_bridge_transfer(Wormhole), andexecute_flash_loaneach minted HBS with zero reference to the documented supply ceiling — for the bridge paths this permanently inflatestotal_supplywith no cap at all. Added the sametotal_shares + minted_amount > MAX_HBS_SUPPLYcheck already used indepositto all three mint sites.Duplicated multisig-approval logic (#459)
investment_vaultandproject_registryeach carried a byte-for-byte-identical private implementation ofvalidate_multisig_config/require_admin_approval/require_multisig_disabled(~75 lines), differing only in which contract's error enum got raised. Extracted the actual algorithm into a newlibs/multisigcrate returning typedResulterrors; each contract now calls the shared implementation through a thin wrapper mapping the result onto its ownVaultError/RegistryError— on-chain error codes are unchanged for both contracts.Why these are safe, minimal fixes
All four are validation-only additions or a pure logic extraction — no changes to any existing authorized behavior's happy path, no new storage layout, no changes to any read path or event schema.
stellar contract buildpasses clean across the whole workspace.Closes
Closes #456
Closes #457
Closes #458
Closes #459
Test plan
stellar contract build(full workspace, wasm32v1-none) — passes cleancargo test -p investment-vault -p project-registry -p multisig— same 15 pre-existing failures as onmain(allDepositLocked/withdraw-lock related, verified unrelated to this change by re-running against a clean checkout), no new failures introduced