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

refactor(engine)!: substate address based on entity id prefix #951

Merged
merged 8 commits into from
Mar 6, 2024

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Mar 1, 2024

Description

Prefix all substate addresses with an entity ID to group objects related to a particular component in the same shard
All shards commit transaction receipt
Adds CreateAccount command which uses the owner public key prefix as the entity ID
Adds OwnerRule::ByPublicKey

Motivation and Context

Substate addresses for Components, Resources, Vaults and NFTs are composed of the entity id (20 bytes), componentKey (8 bytes) and version (4 bytes) (4.29 billion versions).
CreateAccount is implemented the same as CallFunction but sets the entity ID to the owner public key for the call. This can be later changed to create the account without invoking WASM

These "versionable" substate IDs are now 28 bytes long (entity id + component key).

Transaction receipts are committed by all involved shards.

How Has This Been Tested?

CI, Manually

What process can a PR reviewer use to test or verify this change?

Verify that all objects created by a template/component call share the same prefix.

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Please specify

BREAKING CHANGE: all SubstateIds are shorter which breaks all wasm templates (need to recompile), new instruction is not recognised by older versions.

Copy link

github-actions bot commented Mar 1, 2024

Test Results (CI)

475 tests   - 19   471 ✅  - 23   2h 50m 9s ⏱️ + 1h 2m 55s
 48 suites  - 12     0 💤 ± 0 
  1 files    -  1     4 ❌ + 4 

For more details on these failures, see this check.

Results for commit 1ce401b. ± Comparison against base commit 0deb6cd.

This pull request removes 24 and adds 5 tests. Note that renamed tests count towards both.
Scenario: Claim and transfer confidential assets via wallet daemon: tests/features/wallet_daemon.feature:89:3
Scenario: Claim base layer burn funds with wallet daemon: tests/features/claim_burn.feature:6:3
Scenario: Claim validator fees: tests/features/claim_fees.feature:6:3
Scenario: Concurrent calls to the Counter template: tests/features/concurrency.feature:7:3
Scenario: Confidential transfer to unexisting account: tests/features/transfer.feature:163:3
Scenario: Counter template registration and invocation: tests/features/counter.feature:7:3
Scenario: Create account and transfer faucets via wallet daemon: tests/features/wallet_daemon.feature:7:3
Scenario: Create and mint account NFT: tests/features/wallet_daemon.feature:133:3
Scenario: Create resource and mint in one transaction: tests/features/nft.feature:73:3
Scenario: Double Claim base layer burn funds with wallet daemon. should fail: tests/features/claim_burn.feature:44:3
…
tari_engine_types ‑ id_provider::tests::get_random_bytes
tari_engine_types ‑ id_provider::tests::it_fails_if_generating_more_ids_than_the_max
tari_template_lib ‑ models::entity_id::export_bindings_componentkey
tari_template_lib ‑ models::entity_id::export_bindings_entityid
tari_transaction ‑ transaction::export_bindings_versionedsubstate

♻️ This comment has been updated with latest results.

@sdbondi sdbondi force-pushed the engine-entity-id branch 2 times, most recently from 9168a84 to 48e21d3 Compare March 4, 2024 13:07
sdbondi added a commit to tari-project/test-templates that referenced this pull request Mar 4, 2024
@sdbondi sdbondi marked this pull request as ready for review March 5, 2024 05:32
stringhandler
stringhandler previously approved these changes Mar 6, 2024
Copy link
Contributor

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

tested locally and it's working as expected

@stringhandler stringhandler added this pull request to the merge queue Mar 6, 2024
Merged via the queue into tari-project:development with commit 1acbfa9 Mar 6, 2024
9 of 11 checks passed
sdbondi added a commit to sdbondi/tari-dan that referenced this pull request Mar 7, 2024
* development:
  refactor(engine)!: substate address based on entity id prefix (tari-project#951)
sdbondi added a commit to sdbondi/tari-dan that referenced this pull request Mar 8, 2024
* development:
  feat: change bindings package usage to git instead of local (tari-project#963)
  v0.4.1
  fix(engine): claiming burns/fees call frame error (tari-project#960)
  ci: fix windows and macos builds (tari-project#961)
  v0.4.0
  ci: add tari generate to built binaries (tari-project#959)
  refactor(engine)!: substate address based on entity id prefix (tari-project#951)
  fix(walletd): emit AccountChanged event on changes for account refresh (tari-project#956)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants