Support Dynamic Resources in Static Analyzer#2090
Open
BusyWritingCode wants to merge 7 commits intodevelopfrom
Open
Support Dynamic Resources in Static Analyzer#2090BusyWritingCode wants to merge 7 commits intodevelopfrom
BusyWritingCode wants to merge 7 commits intodevelopfrom
Conversation
|
Docker tags |
|
Docker tags |
Benchmark for 9dbd6a2Click to view benchmark
|
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
1 participant
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
The static resource movements analyzer previously only understood resources with known, pre-existing
ResourceAddressvalues. When a manifest created a new resource within the transaction itself (viaallocate_global_address+ one of the*CreateWithInitialSupplycalls) and then performed operations on it using named addresses (withdrawals, deposits, mints, etc.), the analyzer had no way to track those resources. They fell through to the "unspecified resources" path, which eventually caused a panic:"Account withdraw output should not have unspecified resources".This was hit in production, the radix-engine-toolkit uses this analyzer, and when the Gateway API's aggregator encountered one of these manifests on mainnet, it panicked and stopped aggregating.
What changed
New
AnalyzerResourceAddresstype, A new enum that can represent both static (on-ledger, knownResourceAddress) and dynamic (created within the manifest being analyzed) resources:For dynamic resources, the
blueprint_ididentifies the creating blueprint (e.g.FungibleResourceManager) andnamed_addressties back to the manifest-level named address. This gives us enough information to determine fungibility and track the resource across instructions without knowing its final on-ledger address.All core types updated,
TrackedResources,ResourceBounds,AccountWithdraw,AccountDeposit,NetWithdraws,NetDeposits,ManifestResourceConstraints, and related types all changed fromResourceAddresstoAnalyzerResourceAddress. AFrom<ResourceAddress>impl andimpl Into<AnalyzerResourceAddress>parameters keep the API ergonomic for the common static-address case.Address reservation tracking in the visitor, The
StaticResourceMovementsVisitornow tracks address reservations (mapping reservation -> (blueprint, named_address)) alongside named addresses. This is populated fromOnNewNamedAddressevents when the address has an associated reservation.Effect implementations updated for dynamic resolution, Previously, any invocation with a
ManifestResourceAddress::Named(...)would bail out to "unspecified resources". Now, the analyzer resolves named addresses through the tracked named addresses and address reservations maps to produce a properAnalyzerResourceAddress::Dynamic. This covers:AccountWithdraw,AccountWithdrawNonFungibles,AccountLockFeeAndWithdraw,AccountLockFeeAndWithdrawNonFungiblesAccountLockerRecover,AccountLockerRecoverNonFungibles,AccountLockerClaim,AccountLockerClaimNonFungiblesTwoResourcePoolProtectedWithdraw,MultiResourcePoolProtectedWithdrawFungibleResourceManagerCreateWithInitialSupply,NonFungibleResourceManagerCreateWithInitialSupply,NonFungibleResourceManagerCreateRuidWithInitialSupply, these were previously lumped in with "unknown output" invocations and now have proper implementations that resolve the address reservation to a dynamic resource addressFungibleResourceManagerMint,NonFungibleResourceManagerMint,NonFungibleResourceManagerMintRuid,NonFungibleResourceManagerMintSingleRuid, when called on a named address receiver, these now produce tracked output with the correct dynamic addressIf a named address can't be resolved (i.e. it wasn't set up through
allocate_global_address), the analyzer gracefully falls back to the existing "unspecified resources" behavior.Testing
dynamic_resource_create_deposit_withdraw_redeposit_is_correctly_classified) that exercises the full lifecycle: allocate a global address, create a fungible resource with initial supply using that reservation, deposit to an account, withdraw via the named address, and redeposit. The test verifies that all withdrawals, deposits, net withdrawals, and net deposits are correctly tracked usingAnalyzerResourceAddress::Dynamic.AnalyzerResourceAddress(adding.into()conversions where needed) and continue to pass.