From f0a839f874d7b353253a712d9f05299a74fe775d Mon Sep 17 00:00:00 2001 From: hayes-mysten <135670682+hayes-mysten@users.noreply.github.com> Date: Wed, 29 May 2024 12:58:15 -0700 Subject: [PATCH 01/17] fix serialization of v1 objects (#17974) ## Description Describe the changes or additions included in this PR. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .changeset/hot-experts-whisper.md | 5 +++ sdk/typescript/src/transactions/data/v1.ts | 40 ++++++++++++---------- 2 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 .changeset/hot-experts-whisper.md diff --git a/.changeset/hot-experts-whisper.md b/.changeset/hot-experts-whisper.md new file mode 100644 index 0000000000000..107ca959c74df --- /dev/null +++ b/.changeset/hot-experts-whisper.md @@ -0,0 +1,5 @@ +--- +'@mysten/sui': patch +--- + +Fix serialization bug when converting object inputs to v1 JSON diff --git a/sdk/typescript/src/transactions/data/v1.ts b/sdk/typescript/src/transactions/data/v1.ts index a9a10ae7ecc02..90d2d44c308f7 100644 --- a/sdk/typescript/src/transactions/data/v1.ts +++ b/sdk/typescript/src/transactions/data/v1.ts @@ -195,25 +195,27 @@ export function serializeV1TransactionData( return { kind: 'Input', index, - value: input.Object.ImmOrOwnedObject - ? { - ImmOrOwnedObject: input.Object.ImmOrOwnedObject, - } - : input.Object.Receiving - ? { - Receiving: { - digest: input.Object.Receiving.digest, - version: input.Object.Receiving.version, - objectId: input.Object.Receiving.objectId, - }, - } - : { - SharedObject: { - mutable: input.Object.SharedObject.mutable, - initialSharedVersion: input.Object.SharedObject.initialSharedVersion, - objectId: input.Object.SharedObject.objectId, - }, - }, + value: { + Object: input.Object.ImmOrOwnedObject + ? { + ImmOrOwnedObject: input.Object.ImmOrOwnedObject, + } + : input.Object.Receiving + ? { + Receiving: { + digest: input.Object.Receiving.digest, + version: input.Object.Receiving.version, + objectId: input.Object.Receiving.objectId, + }, + } + : { + SharedObject: { + mutable: input.Object.SharedObject.mutable, + initialSharedVersion: input.Object.SharedObject.initialSharedVersion, + objectId: input.Object.SharedObject.objectId, + }, + }, + }, type: 'object', }; } From 3a1da192bd4dd4b3c6cdab1c1fc4913b475a6ead Mon Sep 17 00:00:00 2001 From: "sui-merge-bot[bot]" <114704316+sui-merge-bot[bot]@users.noreply.github.com> Date: Wed, 29 May 2024 20:16:10 +0000 Subject: [PATCH 02/17] Version Packages (#17975) This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## mev-bot@1.0.3 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/create-dapp@0.3.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 - @mysten/dapp-kit@0.14.2 ## @mysten/dapp-kit@0.14.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 - @mysten/wallet-standard@0.12.2 - @mysten/zksend@0.9.2 ## @mysten/deepbook@0.8.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/enoki@0.3.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 - @mysten/zklogin@0.7.2 ## @mysten/graphql-transport@0.2.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/kiosk@0.9.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/suins-toolkit@0.5.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/sui@1.0.2 ### Patch Changes - f0a839f874: Fix serialization bug when converting object inputs to v1 JSON ## @mysten/wallet-standard@0.12.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/zklogin@0.7.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## @mysten/zksend@0.9.2 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 - @mysten/wallet-standard@0.12.2 ## escrow-api-demo@1.0.3 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 ## frontend@0.0.3 ### Patch Changes - Updated dependencies [f0a839f874] - @mysten/sui@1.0.2 - @mysten/dapp-kit@0.14.2 Co-authored-by: github-actions[bot] --- .changeset/hot-experts-whisper.md | 5 ----- examples/mev_bot/CHANGELOG.md | 7 +++++++ examples/mev_bot/package.json | 2 +- examples/trading/api/CHANGELOG.md | 7 +++++++ examples/trading/api/package.json | 2 +- examples/trading/frontend/CHANGELOG.md | 8 ++++++++ examples/trading/frontend/package.json | 2 +- sdk/create-dapp/CHANGELOG.md | 8 ++++++++ sdk/create-dapp/package.json | 2 +- sdk/dapp-kit/CHANGELOG.md | 9 +++++++++ sdk/dapp-kit/package.json | 2 +- sdk/deepbook/CHANGELOG.md | 7 +++++++ sdk/deepbook/package.json | 2 +- sdk/enoki/CHANGELOG.md | 8 ++++++++ sdk/enoki/package.json | 2 +- sdk/graphql-transport/CHANGELOG.md | 7 +++++++ sdk/graphql-transport/package.json | 2 +- sdk/kiosk/CHANGELOG.md | 7 +++++++ sdk/kiosk/package.json | 2 +- sdk/suins-toolkit/CHANGELOG.md | 7 +++++++ sdk/suins-toolkit/package.json | 2 +- sdk/typescript/CHANGELOG.md | 6 ++++++ sdk/typescript/package.json | 2 +- sdk/typescript/src/version.ts | 2 +- sdk/wallet-standard/CHANGELOG.md | 7 +++++++ sdk/wallet-standard/package.json | 2 +- sdk/zklogin/CHANGELOG.md | 7 +++++++ sdk/zklogin/package.json | 2 +- sdk/zksend/CHANGELOG.md | 8 ++++++++ sdk/zksend/package.json | 2 +- 30 files changed, 118 insertions(+), 20 deletions(-) delete mode 100644 .changeset/hot-experts-whisper.md diff --git a/.changeset/hot-experts-whisper.md b/.changeset/hot-experts-whisper.md deleted file mode 100644 index 107ca959c74df..0000000000000 --- a/.changeset/hot-experts-whisper.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@mysten/sui': patch ---- - -Fix serialization bug when converting object inputs to v1 JSON diff --git a/examples/mev_bot/CHANGELOG.md b/examples/mev_bot/CHANGELOG.md index 5d96754bc2a77..b6d5caf88a929 100644 --- a/examples/mev_bot/CHANGELOG.md +++ b/examples/mev_bot/CHANGELOG.md @@ -1,5 +1,12 @@ # mev-bot +## 1.0.3 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 1.0.2 ### Patch Changes diff --git a/examples/mev_bot/package.json b/examples/mev_bot/package.json index d623f243a2fac..1c9f549af5d29 100644 --- a/examples/mev_bot/package.json +++ b/examples/mev_bot/package.json @@ -1,6 +1,6 @@ { "name": "mev-bot", - "version": "1.0.2", + "version": "1.0.3", "main": "src/index.ts", "type": "module", "author": "Patrick ", diff --git a/examples/trading/api/CHANGELOG.md b/examples/trading/api/CHANGELOG.md index 7359c96ee65af..afd546659e2e6 100644 --- a/examples/trading/api/CHANGELOG.md +++ b/examples/trading/api/CHANGELOG.md @@ -1,5 +1,12 @@ # escrow-api-demo +## 1.0.3 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 1.0.2 ### Patch Changes diff --git a/examples/trading/api/package.json b/examples/trading/api/package.json index 48127536a7205..bafecd9fe8c94 100644 --- a/examples/trading/api/package.json +++ b/examples/trading/api/package.json @@ -1,6 +1,6 @@ { "name": "escrow-api-demo", - "version": "1.0.2", + "version": "1.0.3", "private": true, "description": "Demo package for escrow", "main": "server.ts", diff --git a/examples/trading/frontend/CHANGELOG.md b/examples/trading/frontend/CHANGELOG.md index 6adb65ee4857a..8b033ce9c35dd 100644 --- a/examples/trading/frontend/CHANGELOG.md +++ b/examples/trading/frontend/CHANGELOG.md @@ -1,5 +1,13 @@ # frontend +## 0.0.3 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + - @mysten/dapp-kit@0.14.2 + ## 0.0.2 ### Patch Changes diff --git a/examples/trading/frontend/package.json b/examples/trading/frontend/package.json index d32ea88fec824..15368732579cd 100644 --- a/examples/trading/frontend/package.json +++ b/examples/trading/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.0.2", + "version": "0.0.3", "type": "module", "license": "Apache-2.0", "scripts": { diff --git a/sdk/create-dapp/CHANGELOG.md b/sdk/create-dapp/CHANGELOG.md index dd505ed9a8573..20c03ee7ba118 100644 --- a/sdk/create-dapp/CHANGELOG.md +++ b/sdk/create-dapp/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/create-dapp +## 0.3.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + - @mysten/dapp-kit@0.14.2 + ## 0.3.1 ### Patch Changes diff --git a/sdk/create-dapp/package.json b/sdk/create-dapp/package.json index 19f94ca7a517a..7d557162e420b 100644 --- a/sdk/create-dapp/package.json +++ b/sdk/create-dapp/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "A CLI for creating new Sui dApps", "homepage": "https://sdk.mystenlabs.com", - "version": "0.3.1", + "version": "0.3.2", "license": "Apache-2.0", "files": [ "CHANGELOG.md", diff --git a/sdk/dapp-kit/CHANGELOG.md b/sdk/dapp-kit/CHANGELOG.md index 081ac800f89bf..83e1f41eaaa1e 100644 --- a/sdk/dapp-kit/CHANGELOG.md +++ b/sdk/dapp-kit/CHANGELOG.md @@ -1,5 +1,14 @@ # @mysten/dapp-kit +## 0.14.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + - @mysten/wallet-standard@0.12.2 + - @mysten/zksend@0.9.2 + ## 0.14.1 ### Patch Changes diff --git a/sdk/dapp-kit/package.json b/sdk/dapp-kit/package.json index 77b268c2a1f8a..c8fd932372788 100644 --- a/sdk/dapp-kit/package.json +++ b/sdk/dapp-kit/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "A collection of React hooks and components for interacting with the Sui blockchain and wallets.", "homepage": "https://sdk.mystenlabs.com/typescript", - "version": "0.14.1", + "version": "0.14.2", "license": "Apache-2.0", "files": [ "CHANGELOG.md", diff --git a/sdk/deepbook/CHANGELOG.md b/sdk/deepbook/CHANGELOG.md index ebef465cd3ca1..85be49ef1fc26 100644 --- a/sdk/deepbook/CHANGELOG.md +++ b/sdk/deepbook/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/deepbook +## 0.8.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.8.1 ### Patch Changes diff --git a/sdk/deepbook/package.json b/sdk/deepbook/package.json index 186361468378e..0c4e1000a78c3 100644 --- a/sdk/deepbook/package.json +++ b/sdk/deepbook/package.json @@ -2,7 +2,7 @@ "name": "@mysten/deepbook", "author": "Mysten Labs ", "description": "Sui Deepbook SDK", - "version": "0.8.1", + "version": "0.8.2", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/enoki/CHANGELOG.md b/sdk/enoki/CHANGELOG.md index 0f9c6f5f2f04c..f8b6b01432e45 100644 --- a/sdk/enoki/CHANGELOG.md +++ b/sdk/enoki/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/enoki +## 0.3.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + - @mysten/zklogin@0.7.2 + ## 0.3.1 ### Patch Changes diff --git a/sdk/enoki/package.json b/sdk/enoki/package.json index 83a8e755c6562..0c5abb6722823 100644 --- a/sdk/enoki/package.json +++ b/sdk/enoki/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/enoki", - "version": "0.3.1", + "version": "0.3.2", "description": "TODO: Description", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/graphql-transport/CHANGELOG.md b/sdk/graphql-transport/CHANGELOG.md index 79c23e39ec996..9566af2f87a13 100644 --- a/sdk/graphql-transport/CHANGELOG.md +++ b/sdk/graphql-transport/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/graphql-transport +## 0.2.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.2.1 ### Patch Changes diff --git a/sdk/graphql-transport/package.json b/sdk/graphql-transport/package.json index 29378266834bb..b78329e3b5057 100644 --- a/sdk/graphql-transport/package.json +++ b/sdk/graphql-transport/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/graphql-transport", - "version": "0.2.1", + "version": "0.2.2", "description": "A GraphQL transport to allow SuiClient to work with RPC 2.0", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/kiosk/CHANGELOG.md b/sdk/kiosk/CHANGELOG.md index 02d3fc384b331..ef590f999f942 100644 --- a/sdk/kiosk/CHANGELOG.md +++ b/sdk/kiosk/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/kiosk +## 0.9.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.9.1 ### Patch Changes diff --git a/sdk/kiosk/package.json b/sdk/kiosk/package.json index 57ef6898b6868..a08906441c3e4 100644 --- a/sdk/kiosk/package.json +++ b/sdk/kiosk/package.json @@ -2,7 +2,7 @@ "name": "@mysten/kiosk", "author": "Mysten Labs ", "description": "Sui Kiosk library", - "version": "0.9.1", + "version": "0.9.2", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/suins-toolkit/CHANGELOG.md b/sdk/suins-toolkit/CHANGELOG.md index 56bcdc440b961..32bdd4891568a 100644 --- a/sdk/suins-toolkit/CHANGELOG.md +++ b/sdk/suins-toolkit/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/suins-toolkit +## 0.5.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.5.1 ### Patch Changes diff --git a/sdk/suins-toolkit/package.json b/sdk/suins-toolkit/package.json index dc36d67439225..6eb0ae242edec 100644 --- a/sdk/suins-toolkit/package.json +++ b/sdk/suins-toolkit/package.json @@ -2,7 +2,7 @@ "name": "@mysten/suins-toolkit", "author": "Mysten Labs ", "description": "SuiNS TypeScript SDK", - "version": "0.5.1", + "version": "0.5.2", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/typescript/CHANGELOG.md b/sdk/typescript/CHANGELOG.md index 404ed3a4db849..e2eef1a61df96 100644 --- a/sdk/typescript/CHANGELOG.md +++ b/sdk/typescript/CHANGELOG.md @@ -1,5 +1,11 @@ # @mysten/sui.js +## 1.0.2 + +### Patch Changes + +- f0a839f874: Fix serialization bug when converting object inputs to v1 JSON + ## 1.0.1 ### Patch Changes diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index fb62fb28fb3e1..d0144e2d46a0e 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "Sui TypeScript API(Work in Progress)", "homepage": "https://sdk.mystenlabs.com", - "version": "1.0.1", + "version": "1.0.2", "license": "Apache-2.0", "sideEffects": false, "files": [ diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 5449cc3286106..84860d009bec5 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -3,5 +3,5 @@ // This file is generated by genversion.mjs. Do not edit it directly. -export const PACKAGE_VERSION = '1.0.1'; +export const PACKAGE_VERSION = '1.0.2'; export const TARGETED_RPC_VERSION = '1.27.0'; diff --git a/sdk/wallet-standard/CHANGELOG.md b/sdk/wallet-standard/CHANGELOG.md index 21c7972961a51..6dafa1f45bda8 100644 --- a/sdk/wallet-standard/CHANGELOG.md +++ b/sdk/wallet-standard/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/wallet-standard +## 0.12.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.12.1 ### Patch Changes diff --git a/sdk/wallet-standard/package.json b/sdk/wallet-standard/package.json index e36de7e4cf986..84bf09a06ecca 100644 --- a/sdk/wallet-standard/package.json +++ b/sdk/wallet-standard/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/wallet-standard", - "version": "0.12.1", + "version": "0.12.2", "description": "A suite of standard utilities for implementing wallets based on the Wallet Standard.", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/zklogin/CHANGELOG.md b/sdk/zklogin/CHANGELOG.md index 12eff8ac47b5b..88d2453f31f3d 100644 --- a/sdk/zklogin/CHANGELOG.md +++ b/sdk/zklogin/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/zklogin +## 0.7.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + ## 0.7.1 ### Patch Changes diff --git a/sdk/zklogin/package.json b/sdk/zklogin/package.json index baa3900c98b3a..8b3bc096ddab7 100644 --- a/sdk/zklogin/package.json +++ b/sdk/zklogin/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/zklogin", - "version": "0.7.1", + "version": "0.7.2", "description": "Utilities for interacting with zkLogin in Sui", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/zksend/CHANGELOG.md b/sdk/zksend/CHANGELOG.md index a0bb5a4b478a7..30be2608fbd2f 100644 --- a/sdk/zksend/CHANGELOG.md +++ b/sdk/zksend/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/zksend +## 0.9.2 + +### Patch Changes + +- Updated dependencies [f0a839f874] + - @mysten/sui@1.0.2 + - @mysten/wallet-standard@0.12.2 + ## 0.9.1 ### Patch Changes diff --git a/sdk/zksend/package.json b/sdk/zksend/package.json index 301c5eb05f816..03575a07e7257 100644 --- a/sdk/zksend/package.json +++ b/sdk/zksend/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/zksend", - "version": "0.9.1", + "version": "0.9.2", "description": "TODO: Write Description", "license": "Apache-2.0", "author": "Mysten Labs ", From cfb4455ae08e621dcc6c156d70fa6fa0440e5e8b Mon Sep 17 00:00:00 2001 From: mwtian <81660174+mwtian@users.noreply.github.com> Date: Wed, 29 May 2024 13:47:35 -0700 Subject: [PATCH 03/17] [Consensus] remove verbose logging of raw blocks (#17971) ## Description Logging raw block bytes is too verbose for validators lagging in timestamp or accepted blocks. ## Test plan CI --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- consensus/core/src/subscriber.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/consensus/core/src/subscriber.rs b/consensus/core/src/subscriber.rs index 4ca266baa4dda..e9e10797519d2 100644 --- a/consensus/core/src/subscriber.rs +++ b/consensus/core/src/subscriber.rs @@ -14,6 +14,7 @@ use crate::{ block::BlockAPI as _, context::Context, dag_state::DagState, + error::ConsensusError, network::{NetworkClient, NetworkService}, Round, }; @@ -170,10 +171,17 @@ impl Subscriber { .handle_send_block(peer, block.clone()) .await; if let Err(e) = result { - info!( - "Failed to process block from peer {}: {}. Block: {:?}", - peer, e, block, - ); + match e { + ConsensusError::BlockRejected { block_ref, reason } => { + debug!( + "Failed to process block from peer {} for block {:?}: {}", + peer, block_ref, reason + ); + } + _ => { + info!("Invalid block received from peer {}: {}", peer, e,); + } + } } // Reset retries when a block is received. retries = 0; From 41d0cd78796f218160ad23296213e64c662b4e1d Mon Sep 17 00:00:00 2001 From: Andrew Schran Date: Wed, 29 May 2024 17:18:39 -0400 Subject: [PATCH 04/17] add oldest pending round to random beacon backlog error message (#17925) --- crates/sui-network/src/randomness/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/sui-network/src/randomness/mod.rs b/crates/sui-network/src/randomness/mod.rs index 56a509b7b8ede..bfe42ae948405 100644 --- a/crates/sui-network/src/randomness/mod.rs +++ b/crates/sui-network/src/randomness/mod.rs @@ -769,8 +769,16 @@ impl RandomnessEventLoop { if num_rounds_pending / 100 > prev_value / 100 { warn!( // Recording multiples of 100 so tests can match on the log message. - "RandomnessEventLoop randomness generation backlog: over {} rounds are pending", - (num_rounds_pending / 100) * 100 + "RandomnessEventLoop randomness generation backlog: over {} rounds are pending (oldest is {:?})", + (num_rounds_pending / 100) * 100, + match (self.pending_tasks.first(), self.send_tasks.first_key_value()) { + (Some(p), Some((s, _))) => { + std::cmp::min(p, s) + } + (Some(p), None) => p, + (None, Some((s, _))) => s, + (None, None) => &(0, RandomnessRound(0)), + }, ); } self.metrics.set_num_rounds_pending(num_rounds_pending); From 88e9c214fb3e4e7ae75d09f2801c7a66d7522ad4 Mon Sep 17 00:00:00 2001 From: Mark Logan <103447440+mystenmark@users.noreply.github.com> Date: Wed, 29 May 2024 14:22:54 -0700 Subject: [PATCH 05/17] Ensure that surfer cannot get stuck forever on error retry (#17778) --- crates/sui-surfer/src/surf_strategy.rs | 12 ++++-------- crates/sui-surfer/src/surfer_task.rs | 15 ++++++++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/sui-surfer/src/surf_strategy.rs b/crates/sui-surfer/src/surf_strategy.rs index 05bb0d3af3154..4747b25b18ca4 100644 --- a/crates/sui-surfer/src/surf_strategy.rs +++ b/crates/sui-surfer/src/surf_strategy.rs @@ -10,7 +10,7 @@ use sui_types::{ base_types::ObjectRef, transaction::{CallArg, ObjectArg}, }; -use tokio::{sync::watch, time::Instant}; +use tokio::time::Instant; use tracing::debug; use crate::surfer_state::{EntryFunction, SurferState}; @@ -32,14 +32,13 @@ impl SurfStrategy { } /// Given a state and a list of callable Move entry functions, - /// explore them for a while, and eventually return. It's important that it - /// eventually returns just so that the runtime can perform a few tasks - /// such as checking whether to exit, or to sync some global state if need to. + /// explore them for a while, and eventually return. This function may + /// not return in some situations, so its important to call it with a + /// timeout or select! to ensure the task doesn't block forever. pub async fn surf_for_a_while( &mut self, state: &mut SurferState, mut entry_functions: Vec, - exit: &watch::Receiver<()>, ) { entry_functions.shuffle(&mut state.rng); for entry in entry_functions { @@ -54,9 +53,6 @@ impl SurfStrategy { state .execute_move_transaction(entry.package, entry.module, entry.function, args) .await; - if exit.has_changed().unwrap() { - return; - } tokio::time::sleep_until(next_tx_time).await; } } diff --git a/crates/sui-surfer/src/surfer_task.rs b/crates/sui-surfer/src/surfer_task.rs index 6eb5af71f2ae5..e7d43cafd3567 100644 --- a/crates/sui-surfer/src/surfer_task.rs +++ b/crates/sui-surfer/src/surfer_task.rs @@ -128,11 +128,16 @@ impl SurferTask { pub async fn surf(mut self) -> SurfStatistics { loop { let entry_functions = self.state.entry_functions.read().await.clone(); - self.surf_strategy - .surf_for_a_while(&mut self.state, entry_functions, &self.exit_rcv) - .await; - if self.exit_rcv.has_changed().unwrap() { - return self.state.stats; + + tokio::select! { + _ = self.surf_strategy + .surf_for_a_while(&mut self.state, entry_functions) => { + continue; + } + + _ = self.exit_rcv.changed() => { + return self.state.stats; + } } } } From 1f205808416ecb69daea7c13322c5ad9ffa1160f Mon Sep 17 00:00:00 2001 From: hayes-mysten <135670682+hayes-mysten@users.noreply.github.com> Date: Wed, 29 May 2024 14:35:22 -0700 Subject: [PATCH 06/17] Fix parsing of object refs from v1 json (#17979) ## Description Describe the changes or additions included in this PR. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .changeset/rare-oranges-crash.md | 5 + .../src/transactions/data/internal.ts | 2 +- sdk/typescript/src/transactions/data/v1.ts | 65 +++- sdk/typescript/src/transactions/data/v2.ts | 2 +- sdk/typescript/test/unit/v1-json.test.ts | 326 ++++++++++++++++++ 5 files changed, 387 insertions(+), 13 deletions(-) create mode 100644 .changeset/rare-oranges-crash.md create mode 100644 sdk/typescript/test/unit/v1-json.test.ts diff --git a/.changeset/rare-oranges-crash.md b/.changeset/rare-oranges-crash.md new file mode 100644 index 0000000000000..0f2622cf29b41 --- /dev/null +++ b/.changeset/rare-oranges-crash.md @@ -0,0 +1,5 @@ +--- +'@mysten/sui': patch +--- + +Fix parsing of object refs from v1 json diff --git a/sdk/typescript/src/transactions/data/internal.ts b/sdk/typescript/src/transactions/data/internal.ts index 3102438866b02..3bb894c3e3dd1 100644 --- a/sdk/typescript/src/transactions/data/internal.ts +++ b/sdk/typescript/src/transactions/data/internal.ts @@ -273,7 +273,7 @@ export const ObjectArg = safeEnum({ objectId: ObjectID, // snake case in rust initialSharedVersion: JsonU64, - mutable: nullable(boolean()), + mutable: boolean(), }), Receiving: ObjectRef, }); diff --git a/sdk/typescript/src/transactions/data/v1.ts b/sdk/typescript/src/transactions/data/v1.ts index 90d2d44c308f7..225efbee1594e 100644 --- a/sdk/typescript/src/transactions/data/v1.ts +++ b/sdk/typescript/src/transactions/data/v1.ts @@ -6,6 +6,7 @@ import type { BaseSchema, Input, Output } from 'valibot'; import { array, bigint, + boolean, custom, integer, is, @@ -24,20 +25,30 @@ import { import { TypeTagSerializer } from '../../bcs/index.js'; import type { StructTag as StructTagType, TypeTag as TypeTagType } from '../../bcs/types.js'; -import { ObjectArg, safeEnum, TransactionData } from './internal.js'; +import { JsonU64, ObjectID, safeEnum, TransactionData } from './internal.js'; import type { Argument } from './internal.js'; -export const NormalizedCallArg = safeEnum({ - Object: ObjectArg, - Pure: array(number([integer()])), -}); - export const ObjectRef = object({ digest: string(), objectId: string(), version: union([number([integer()]), string(), bigint()]), }); +const ObjectArg = safeEnum({ + ImmOrOwned: ObjectRef, + Shared: object({ + objectId: ObjectID, + initialSharedVersion: JsonU64, + mutable: boolean(), + }), + Receiving: ObjectRef, +}); + +export const NormalizedCallArg = safeEnum({ + Object: ObjectArg, + Pure: array(number([integer()])), +}); + const TransactionInput = union([ object({ kind: literal('Input'), @@ -198,7 +209,7 @@ export function serializeV1TransactionData( value: { Object: input.Object.ImmOrOwnedObject ? { - ImmOrOwnedObject: input.Object.ImmOrOwnedObject, + ImmOrOwned: input.Object.ImmOrOwnedObject, } : input.Object.Receiving ? { @@ -209,7 +220,7 @@ export function serializeV1TransactionData( }, } : { - SharedObject: { + Shared: { mutable: input.Object.SharedObject.mutable, initialSharedVersion: input.Object.SharedObject.initialSharedVersion, objectId: input.Object.SharedObject.objectId, @@ -382,9 +393,41 @@ export function transactionDataFromV1(data: SerializedTransactionDataV1): Transa const value = parse(NormalizedCallArg, input.value); if (value.Object) { - return { - Object: value.Object, - }; + if (value.Object.ImmOrOwned) { + return { + Object: { + ImmOrOwnedObject: { + objectId: value.Object.ImmOrOwned.objectId, + version: String(value.Object.ImmOrOwned.version), + digest: value.Object.ImmOrOwned.digest, + }, + }, + }; + } + if (value.Object.Shared) { + return { + Object: { + SharedObject: { + mutable: value.Object.Shared.mutable ?? null, + initialSharedVersion: value.Object.Shared.initialSharedVersion, + objectId: value.Object.Shared.objectId, + }, + }, + }; + } + if (value.Object.Receiving) { + return { + Object: { + Receiving: { + digest: value.Object.Receiving.digest, + version: String(value.Object.Receiving.version), + objectId: value.Object.Receiving.objectId, + }, + }, + }; + } + + throw new Error('Invalid object input'); } return { diff --git a/sdk/typescript/src/transactions/data/v2.ts b/sdk/typescript/src/transactions/data/v2.ts index 990f65eb9e746..b4b07e94cb2a3 100644 --- a/sdk/typescript/src/transactions/data/v2.ts +++ b/sdk/typescript/src/transactions/data/v2.ts @@ -107,7 +107,7 @@ const ObjectArg = enumUnion({ objectId: ObjectID, // snake case in rust initialSharedVersion: JsonU64, - mutable: nullable(boolean()), + mutable: boolean(), }), Receiving: ObjectRef, }); diff --git a/sdk/typescript/test/unit/v1-json.test.ts b/sdk/typescript/test/unit/v1-json.test.ts new file mode 100644 index 0000000000000..7eb31dd1545f2 --- /dev/null +++ b/sdk/typescript/test/unit/v1-json.test.ts @@ -0,0 +1,326 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +import { toB58 } from '@mysten/bcs'; +import { describe, expect, it } from 'vitest'; + +import { Inputs, Transaction } from '../../src/transactions'; + +describe('V1 JSON serialization', () => { + it('can serialize and deserialize transactions', async () => { + const tx = new Transaction(); + + tx.moveCall({ + target: '0x2::foo::bar', + arguments: [ + tx.object('0x123'), + tx.object( + Inputs.ReceivingRef({ + objectId: '1', + version: '123', + digest: toB58(new Uint8Array(32).fill(0x1)), + }), + ), + tx.object( + Inputs.SharedObjectRef({ + objectId: '2', + mutable: true, + initialSharedVersion: '123', + }), + ), + tx.object( + Inputs.ObjectRef({ + objectId: '3', + version: '123', + digest: toB58(new Uint8Array(32).fill(0x1)), + }), + ), + tx.pure.address('0x2'), + ], + }); + + const jsonv2 = await tx.toJSON(); + const jsonv1 = JSON.parse(tx.serialize()); + + expect(jsonv1).toMatchInlineSnapshot(` + { + "expiration": null, + "gasConfig": {}, + "inputs": [ + { + "index": 0, + "kind": "Input", + "type": "object", + "value": "0x0000000000000000000000000000000000000000000000000000000000000123", + }, + { + "index": 1, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "Receiving": { + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi", + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000001", + "version": "123", + }, + }, + }, + }, + { + "index": 2, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "Shared": { + "initialSharedVersion": "123", + "mutable": true, + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000002", + }, + }, + }, + }, + { + "index": 3, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "ImmOrOwned": { + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi", + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000003", + "version": "123", + }, + }, + }, + }, + { + "index": 4, + "kind": "Input", + "type": "pure", + "value": { + "Pure": [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 2, + ], + }, + }, + ], + "transactions": [ + { + "arguments": [ + { + "index": 0, + "kind": "Input", + "type": "object", + "value": "0x0000000000000000000000000000000000000000000000000000000000000123", + }, + { + "index": 1, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "Receiving": { + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi", + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000001", + "version": "123", + }, + }, + }, + }, + { + "index": 2, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "Shared": { + "initialSharedVersion": "123", + "mutable": true, + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000002", + }, + }, + }, + }, + { + "index": 3, + "kind": "Input", + "type": "object", + "value": { + "Object": { + "ImmOrOwned": { + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi", + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000003", + "version": "123", + }, + }, + }, + }, + { + "index": 4, + "kind": "Input", + "type": "pure", + "value": { + "Pure": [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 2, + ], + }, + }, + ], + "kind": "MoveCall", + "target": "0x0000000000000000000000000000000000000000000000000000000000000002::foo::bar", + "typeArguments": [], + }, + ], + "version": 1, + } + `); + + const tx2 = Transaction.from(JSON.stringify(jsonv1)); + + expect(await tx2.toJSON()).toEqual(jsonv2); + + expect(jsonv2).toMatchInlineSnapshot(` + "{ + "version": 2, + "sender": null, + "expiration": null, + "gasData": { + "budget": null, + "price": null, + "owner": null, + "payment": null + }, + "inputs": [ + { + "UnresolvedObject": { + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000123" + } + }, + { + "Object": { + "Receiving": { + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000001", + "version": "123", + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi" + } + } + }, + { + "Object": { + "SharedObject": { + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000002", + "initialSharedVersion": "123", + "mutable": true + } + } + }, + { + "Object": { + "ImmOrOwnedObject": { + "objectId": "0x0000000000000000000000000000000000000000000000000000000000000003", + "version": "123", + "digest": "4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi" + } + } + }, + { + "Pure": { + "bytes": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAI=" + } + } + ], + "commands": [ + { + "MoveCall": { + "package": "0x0000000000000000000000000000000000000000000000000000000000000002", + "module": "foo", + "function": "bar", + "typeArguments": [], + "arguments": [ + { + "Input": 0 + }, + { + "Input": 1 + }, + { + "Input": 2 + }, + { + "Input": 3 + }, + { + "Input": 4 + } + ] + } + } + ] + }" + `); + }); +}); From da4452885cc21e931f3c051dee128cc84b294cc6 Mon Sep 17 00:00:00 2001 From: "sui-merge-bot[bot]" <114704316+sui-merge-bot[bot]@users.noreply.github.com> Date: Wed, 29 May 2024 22:00:56 +0000 Subject: [PATCH 07/17] Version Packages (#17981) This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## mev-bot@1.0.4 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/create-dapp@0.3.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 - @mysten/dapp-kit@0.14.3 ## @mysten/dapp-kit@0.14.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 - @mysten/wallet-standard@0.12.3 - @mysten/zksend@0.9.3 ## @mysten/deepbook@0.8.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/enoki@0.3.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 - @mysten/zklogin@0.7.3 ## @mysten/graphql-transport@0.2.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/kiosk@0.9.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/suins-toolkit@0.5.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/sui@1.0.3 ### Patch Changes - 1f20580841: Fix parsing of object refs from v1 json ## @mysten/wallet-standard@0.12.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/zklogin@0.7.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## @mysten/zksend@0.9.3 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 - @mysten/wallet-standard@0.12.3 ## escrow-api-demo@1.0.4 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 ## frontend@0.0.4 ### Patch Changes - Updated dependencies [1f20580841] - @mysten/sui@1.0.3 - @mysten/dapp-kit@0.14.3 Co-authored-by: github-actions[bot] --- .changeset/rare-oranges-crash.md | 5 ----- examples/mev_bot/CHANGELOG.md | 7 +++++++ examples/mev_bot/package.json | 2 +- examples/trading/api/CHANGELOG.md | 7 +++++++ examples/trading/api/package.json | 2 +- examples/trading/frontend/CHANGELOG.md | 8 ++++++++ examples/trading/frontend/package.json | 2 +- sdk/create-dapp/CHANGELOG.md | 8 ++++++++ sdk/create-dapp/package.json | 2 +- sdk/dapp-kit/CHANGELOG.md | 9 +++++++++ sdk/dapp-kit/package.json | 2 +- sdk/deepbook/CHANGELOG.md | 7 +++++++ sdk/deepbook/package.json | 2 +- sdk/enoki/CHANGELOG.md | 8 ++++++++ sdk/enoki/package.json | 2 +- sdk/graphql-transport/CHANGELOG.md | 7 +++++++ sdk/graphql-transport/package.json | 2 +- sdk/kiosk/CHANGELOG.md | 7 +++++++ sdk/kiosk/package.json | 2 +- sdk/suins-toolkit/CHANGELOG.md | 7 +++++++ sdk/suins-toolkit/package.json | 2 +- sdk/typescript/CHANGELOG.md | 6 ++++++ sdk/typescript/package.json | 2 +- sdk/typescript/src/version.ts | 2 +- sdk/wallet-standard/CHANGELOG.md | 7 +++++++ sdk/wallet-standard/package.json | 2 +- sdk/zklogin/CHANGELOG.md | 7 +++++++ sdk/zklogin/package.json | 2 +- sdk/zksend/CHANGELOG.md | 8 ++++++++ sdk/zksend/package.json | 2 +- 30 files changed, 118 insertions(+), 20 deletions(-) delete mode 100644 .changeset/rare-oranges-crash.md diff --git a/.changeset/rare-oranges-crash.md b/.changeset/rare-oranges-crash.md deleted file mode 100644 index 0f2622cf29b41..0000000000000 --- a/.changeset/rare-oranges-crash.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@mysten/sui': patch ---- - -Fix parsing of object refs from v1 json diff --git a/examples/mev_bot/CHANGELOG.md b/examples/mev_bot/CHANGELOG.md index b6d5caf88a929..afa2506c59025 100644 --- a/examples/mev_bot/CHANGELOG.md +++ b/examples/mev_bot/CHANGELOG.md @@ -1,5 +1,12 @@ # mev-bot +## 1.0.4 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 1.0.3 ### Patch Changes diff --git a/examples/mev_bot/package.json b/examples/mev_bot/package.json index 1c9f549af5d29..bcf3ea1c5b0c9 100644 --- a/examples/mev_bot/package.json +++ b/examples/mev_bot/package.json @@ -1,6 +1,6 @@ { "name": "mev-bot", - "version": "1.0.3", + "version": "1.0.4", "main": "src/index.ts", "type": "module", "author": "Patrick ", diff --git a/examples/trading/api/CHANGELOG.md b/examples/trading/api/CHANGELOG.md index afd546659e2e6..b27e9470af522 100644 --- a/examples/trading/api/CHANGELOG.md +++ b/examples/trading/api/CHANGELOG.md @@ -1,5 +1,12 @@ # escrow-api-demo +## 1.0.4 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 1.0.3 ### Patch Changes diff --git a/examples/trading/api/package.json b/examples/trading/api/package.json index bafecd9fe8c94..847a83edc00cc 100644 --- a/examples/trading/api/package.json +++ b/examples/trading/api/package.json @@ -1,6 +1,6 @@ { "name": "escrow-api-demo", - "version": "1.0.3", + "version": "1.0.4", "private": true, "description": "Demo package for escrow", "main": "server.ts", diff --git a/examples/trading/frontend/CHANGELOG.md b/examples/trading/frontend/CHANGELOG.md index 8b033ce9c35dd..c25eeff2f4bc4 100644 --- a/examples/trading/frontend/CHANGELOG.md +++ b/examples/trading/frontend/CHANGELOG.md @@ -1,5 +1,13 @@ # frontend +## 0.0.4 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + - @mysten/dapp-kit@0.14.3 + ## 0.0.3 ### Patch Changes diff --git a/examples/trading/frontend/package.json b/examples/trading/frontend/package.json index 15368732579cd..3005362cd3175 100644 --- a/examples/trading/frontend/package.json +++ b/examples/trading/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.0.3", + "version": "0.0.4", "type": "module", "license": "Apache-2.0", "scripts": { diff --git a/sdk/create-dapp/CHANGELOG.md b/sdk/create-dapp/CHANGELOG.md index 20c03ee7ba118..99fe3cbd908b7 100644 --- a/sdk/create-dapp/CHANGELOG.md +++ b/sdk/create-dapp/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/create-dapp +## 0.3.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + - @mysten/dapp-kit@0.14.3 + ## 0.3.2 ### Patch Changes diff --git a/sdk/create-dapp/package.json b/sdk/create-dapp/package.json index 7d557162e420b..5c2a45db0d051 100644 --- a/sdk/create-dapp/package.json +++ b/sdk/create-dapp/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "A CLI for creating new Sui dApps", "homepage": "https://sdk.mystenlabs.com", - "version": "0.3.2", + "version": "0.3.3", "license": "Apache-2.0", "files": [ "CHANGELOG.md", diff --git a/sdk/dapp-kit/CHANGELOG.md b/sdk/dapp-kit/CHANGELOG.md index 83e1f41eaaa1e..79ff9e7d4ecdb 100644 --- a/sdk/dapp-kit/CHANGELOG.md +++ b/sdk/dapp-kit/CHANGELOG.md @@ -1,5 +1,14 @@ # @mysten/dapp-kit +## 0.14.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + - @mysten/wallet-standard@0.12.3 + - @mysten/zksend@0.9.3 + ## 0.14.2 ### Patch Changes diff --git a/sdk/dapp-kit/package.json b/sdk/dapp-kit/package.json index c8fd932372788..4f05b67087bac 100644 --- a/sdk/dapp-kit/package.json +++ b/sdk/dapp-kit/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "A collection of React hooks and components for interacting with the Sui blockchain and wallets.", "homepage": "https://sdk.mystenlabs.com/typescript", - "version": "0.14.2", + "version": "0.14.3", "license": "Apache-2.0", "files": [ "CHANGELOG.md", diff --git a/sdk/deepbook/CHANGELOG.md b/sdk/deepbook/CHANGELOG.md index 85be49ef1fc26..e9ba1a5096157 100644 --- a/sdk/deepbook/CHANGELOG.md +++ b/sdk/deepbook/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/deepbook +## 0.8.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.8.2 ### Patch Changes diff --git a/sdk/deepbook/package.json b/sdk/deepbook/package.json index 0c4e1000a78c3..9d0c269f2fb2d 100644 --- a/sdk/deepbook/package.json +++ b/sdk/deepbook/package.json @@ -2,7 +2,7 @@ "name": "@mysten/deepbook", "author": "Mysten Labs ", "description": "Sui Deepbook SDK", - "version": "0.8.2", + "version": "0.8.3", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/enoki/CHANGELOG.md b/sdk/enoki/CHANGELOG.md index f8b6b01432e45..82de813f25085 100644 --- a/sdk/enoki/CHANGELOG.md +++ b/sdk/enoki/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/enoki +## 0.3.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + - @mysten/zklogin@0.7.3 + ## 0.3.2 ### Patch Changes diff --git a/sdk/enoki/package.json b/sdk/enoki/package.json index 0c5abb6722823..92ebbb89c1e02 100644 --- a/sdk/enoki/package.json +++ b/sdk/enoki/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/enoki", - "version": "0.3.2", + "version": "0.3.3", "description": "TODO: Description", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/graphql-transport/CHANGELOG.md b/sdk/graphql-transport/CHANGELOG.md index 9566af2f87a13..17733bf0ce256 100644 --- a/sdk/graphql-transport/CHANGELOG.md +++ b/sdk/graphql-transport/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/graphql-transport +## 0.2.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.2.2 ### Patch Changes diff --git a/sdk/graphql-transport/package.json b/sdk/graphql-transport/package.json index b78329e3b5057..c396cbed9cc5b 100644 --- a/sdk/graphql-transport/package.json +++ b/sdk/graphql-transport/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/graphql-transport", - "version": "0.2.2", + "version": "0.2.3", "description": "A GraphQL transport to allow SuiClient to work with RPC 2.0", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/kiosk/CHANGELOG.md b/sdk/kiosk/CHANGELOG.md index ef590f999f942..e56b4d922541f 100644 --- a/sdk/kiosk/CHANGELOG.md +++ b/sdk/kiosk/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/kiosk +## 0.9.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.9.2 ### Patch Changes diff --git a/sdk/kiosk/package.json b/sdk/kiosk/package.json index a08906441c3e4..05abcae6317f0 100644 --- a/sdk/kiosk/package.json +++ b/sdk/kiosk/package.json @@ -2,7 +2,7 @@ "name": "@mysten/kiosk", "author": "Mysten Labs ", "description": "Sui Kiosk library", - "version": "0.9.2", + "version": "0.9.3", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/suins-toolkit/CHANGELOG.md b/sdk/suins-toolkit/CHANGELOG.md index 32bdd4891568a..1bae5c959665d 100644 --- a/sdk/suins-toolkit/CHANGELOG.md +++ b/sdk/suins-toolkit/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/suins-toolkit +## 0.5.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.5.2 ### Patch Changes diff --git a/sdk/suins-toolkit/package.json b/sdk/suins-toolkit/package.json index 6eb0ae242edec..bfa8a6aea15bd 100644 --- a/sdk/suins-toolkit/package.json +++ b/sdk/suins-toolkit/package.json @@ -2,7 +2,7 @@ "name": "@mysten/suins-toolkit", "author": "Mysten Labs ", "description": "SuiNS TypeScript SDK", - "version": "0.5.2", + "version": "0.5.3", "license": "Apache-2.0", "type": "commonjs", "main": "./dist/cjs/index.js", diff --git a/sdk/typescript/CHANGELOG.md b/sdk/typescript/CHANGELOG.md index e2eef1a61df96..6811028f1a063 100644 --- a/sdk/typescript/CHANGELOG.md +++ b/sdk/typescript/CHANGELOG.md @@ -1,5 +1,11 @@ # @mysten/sui.js +## 1.0.3 + +### Patch Changes + +- 1f20580841: Fix parsing of object refs from v1 json + ## 1.0.2 ### Patch Changes diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index d0144e2d46a0e..3a32181e562ce 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -3,7 +3,7 @@ "author": "Mysten Labs ", "description": "Sui TypeScript API(Work in Progress)", "homepage": "https://sdk.mystenlabs.com", - "version": "1.0.2", + "version": "1.0.3", "license": "Apache-2.0", "sideEffects": false, "files": [ diff --git a/sdk/typescript/src/version.ts b/sdk/typescript/src/version.ts index 84860d009bec5..c73b23a82c400 100644 --- a/sdk/typescript/src/version.ts +++ b/sdk/typescript/src/version.ts @@ -3,5 +3,5 @@ // This file is generated by genversion.mjs. Do not edit it directly. -export const PACKAGE_VERSION = '1.0.2'; +export const PACKAGE_VERSION = '1.0.3'; export const TARGETED_RPC_VERSION = '1.27.0'; diff --git a/sdk/wallet-standard/CHANGELOG.md b/sdk/wallet-standard/CHANGELOG.md index 6dafa1f45bda8..70638d814ad22 100644 --- a/sdk/wallet-standard/CHANGELOG.md +++ b/sdk/wallet-standard/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/wallet-standard +## 0.12.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.12.2 ### Patch Changes diff --git a/sdk/wallet-standard/package.json b/sdk/wallet-standard/package.json index 84bf09a06ecca..1c79f8c8f0826 100644 --- a/sdk/wallet-standard/package.json +++ b/sdk/wallet-standard/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/wallet-standard", - "version": "0.12.2", + "version": "0.12.3", "description": "A suite of standard utilities for implementing wallets based on the Wallet Standard.", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/zklogin/CHANGELOG.md b/sdk/zklogin/CHANGELOG.md index 88d2453f31f3d..873a6b9ce9946 100644 --- a/sdk/zklogin/CHANGELOG.md +++ b/sdk/zklogin/CHANGELOG.md @@ -1,5 +1,12 @@ # @mysten/zklogin +## 0.7.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + ## 0.7.2 ### Patch Changes diff --git a/sdk/zklogin/package.json b/sdk/zklogin/package.json index 8b3bc096ddab7..4afcbe8304844 100644 --- a/sdk/zklogin/package.json +++ b/sdk/zklogin/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/zklogin", - "version": "0.7.2", + "version": "0.7.3", "description": "Utilities for interacting with zkLogin in Sui", "license": "Apache-2.0", "author": "Mysten Labs ", diff --git a/sdk/zksend/CHANGELOG.md b/sdk/zksend/CHANGELOG.md index 30be2608fbd2f..a8fe9ac818645 100644 --- a/sdk/zksend/CHANGELOG.md +++ b/sdk/zksend/CHANGELOG.md @@ -1,5 +1,13 @@ # @mysten/zksend +## 0.9.3 + +### Patch Changes + +- Updated dependencies [1f20580841] + - @mysten/sui@1.0.3 + - @mysten/wallet-standard@0.12.3 + ## 0.9.2 ### Patch Changes diff --git a/sdk/zksend/package.json b/sdk/zksend/package.json index 03575a07e7257..c8cbcde4f652e 100644 --- a/sdk/zksend/package.json +++ b/sdk/zksend/package.json @@ -1,6 +1,6 @@ { "name": "@mysten/zksend", - "version": "0.9.2", + "version": "0.9.3", "description": "TODO: Write Description", "license": "Apache-2.0", "author": "Mysten Labs ", From 0cd71379109255baca33e37757daabe21decc756 Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Wed, 29 May 2024 18:24:47 -0400 Subject: [PATCH 08/17] doc: update README (#17916) This PR rewords the README.md file. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c03446966cacc..1323054a4f9f8 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Sui is written in [Rust](https://www.rust-lang.org) and supports smart contracts Sui has a native token called SUI, with a fixed supply. The SUI token is used to pay for gas, and is also used as [delegated stake on authorities](https://learn.bybit.com/blockchain/delegated-proof-of-stake-dpos/) within an epoch. The voting power of authorities within this epoch is a function of this delegated stake. Authorities are periodically reconfigured according to the stake delegated to them. In any epoch, the set of authorities is [Byzantine fault tolerant](https://pmg.csail.mit.edu/papers/osdi99.pdf). At the end of the epoch, fees collected through all transactions processed are distributed to authorities according to their contribution to the operation of the system. Authorities can in turn share some of the fees as rewards to users that delegated stakes to them. -Sui is backed by a number of state-of-the-art [peer-reviewed works](https://github.com/MystenLabs/sui/blob/main/docs/content/concepts/research-papers.mdx) and years of open source development. +Sui is supported by several cutting-edge [peer-reviewed studies](https://github.com/MystenLabs/sui/blob/main/docs/content/concepts/research-papers.mdx) and extensive years of open-source development. ## More About Sui From 9f4fabc63375093cdff7037462296792ac738ede Mon Sep 17 00:00:00 2001 From: Cam Swords Date: Wed, 29 May 2024 15:32:17 -0700 Subject: [PATCH 09/17] [move][naming][0/k] Clean up naming infrastructure (#17839) ## Description This cleans up and combines a lot of the code in naming into a more-uniform interface. This work is in service of hopefully one day combining path expansion and name resolution into a single operation. ## Test plan A few of the tests were updated with expected output. Bikeshedding is quite welcome on those, but overall things are more-uniform with more normalized structure now. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .../src/expansion/alias_map_builder.rs | 3 + .../move-compiler/src/naming/translate.rs | 2685 ++++++++++------- .../crates/move-compiler/src/shared/mod.rs | 4 + ...ants_dont_shadow_leading_access_struct.exp | 8 +- .../expansion/use_nested_self_as_invalid.exp | 8 +- .../matching/invalid_head_ctor_3.exp | 4 +- .../naming/clever_errors_line_assert.exp | 7 +- ...ositional_struct_non_positional_unpack.exp | 10 +- ...tional_unpack_of_non_positional_struct.exp | 10 +- .../naming/struct_ellipsis_invalid.exp | 30 +- ...positional_struct_unpack_deeply_nested.exp | 10 +- .../tests/move_2024/parser/long_path.exp | 8 +- ...tional_struct_explicit_type_arg_assign.exp | 2 +- .../positional_struct_explicit_type_args.exp | 2 +- .../move_check/deprecated/assert_function.exp | 7 +- .../invalid_unpack_assign_lhs_other_value.exp | 4 +- .../invalid_unpack_assign_mdot_no_struct.exp | 4 +- ...pack_no_fields_single_block_other_expr.exp | 4 +- .../expansion/unpack_assign_other_expr.exp | 12 +- .../expansion/weird_apply_assign.exp | 2 +- .../move_check/naming/unbound_constant.exp | 4 +- .../move_check/naming/unbound_module_name.exp | 8 +- .../naming/unbound_struct_in_current.exp | 16 +- .../naming/unbound_struct_in_module.exp | 4 +- .../invalid_unpack_assign_rhs_not_fields.exp | 6 +- .../parser/positional_struct_unpack.exp | 5 +- .../typing/module_call_missing_function.exp | 2 +- 27 files changed, 1648 insertions(+), 1221 deletions(-) diff --git a/external-crates/move/crates/move-compiler/src/expansion/alias_map_builder.rs b/external-crates/move/crates/move-compiler/src/expansion/alias_map_builder.rs index 21e914e045b82..30ed68580776e 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/alias_map_builder.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/alias_map_builder.rs @@ -238,6 +238,9 @@ impl AliasMapBuilder { result } + // TODO: the functions below should take a flag indicating if they are from a `use` or local + // definition for better error reporting. + /// Adds a module alias to the map. /// Errors if one already bound for that alias pub fn add_module_alias(&mut self, alias: Name, ident: ModuleIdent) -> Result<(), Loc> { diff --git a/external-crates/move/crates/move-compiler/src/naming/translate.rs b/external-crates/move/crates/move-compiler/src/naming/translate.rs index 1ce2fc283609c..38a68e913601a 100644 --- a/external-crates/move/crates/move-compiler/src/naming/translate.rs +++ b/external-crates/move/crates/move-compiler/src/naming/translate.rs @@ -4,7 +4,11 @@ use crate::{ debug_display, diag, - diagnostics::{self, codes::*}, + diagnostics::{ + self, + codes::{self, *}, + Diagnostic, + }, editions::FeatureGate, expansion::{ ast::{self as E, AbilitySet, Ellipsis, ModuleIdent, Mutability, Visibility}, @@ -34,103 +38,136 @@ use std::{ // Resolver Types //************************************************************************************************** +// ------------------------------------------------------------------------------------------------- +// Module Definition Resolution Types +// ------------------------------------------------------------------------------------------------- +// These type definitions hold the information about module members, which we can retain and reuse. +// These are used to build up the actual resolution types returned during name resolution. + #[derive(Debug, Clone)] -pub(super) enum ResolvedType { - ModuleType(Box), - TParam(Loc, N::TParam), - BuiltinType(N::BuiltinTypeName_), - Hole, // '_' type - Unbound, +pub struct ResolvedModuleFunction { + pub mident: ModuleIdent, + pub name: FunctionName, + pub tyarg_arity: usize, + pub arity: usize, } #[derive(Debug, Clone)] -pub(super) struct ResolvedModuleType { - // original names/locs are provided to preserve loc information if needed - pub original_loc: Loc, - pub original_type_name: Name, - pub module_type: ModuleType, +pub struct ResolvedStruct { + pub mident: ModuleIdent, + pub name: DatatypeName, + pub decl_loc: Loc, + pub tyarg_arity: usize, + pub field_info: FieldInfo, } #[derive(Debug, Clone)] -pub(super) enum ModuleType { - Struct(Box), - Enum(Box), +pub struct ResolvedEnum { + pub mident: ModuleIdent, + pub name: DatatypeName, + pub decl_loc: Loc, + pub tyarg_arity: usize, + pub variants: UniqueMap, } #[derive(Debug, Clone)] -pub(super) struct StructType { - original_mident: ModuleIdent, - decl_loc: Loc, - arity: usize, - field_info: FieldInfo, +pub struct ResolvedVariant { + pub mident: ModuleIdent, + pub enum_name: DatatypeName, + pub tyarg_arity: usize, + pub name: VariantName, + pub decl_loc: Loc, + pub field_info: FieldInfo, } #[derive(Debug, Clone)] -pub(super) struct EnumType { - original_mident: ModuleIdent, - decl_loc: Loc, - arity: usize, - variants: UniqueMap, +pub enum FieldInfo { + Positional(usize), + Named(BTreeSet), + Empty, } #[derive(Debug, Clone)] -#[allow(dead_code)] -pub(super) struct VariantConstructor { - original_variant_name: Name, - decl_loc: Loc, - field_info: FieldInfo, +pub struct ResolvedConstant { + pub mident: ModuleIdent, + pub name: ConstantName, + pub decl_loc: Loc, } #[derive(Debug, Clone)] -pub(super) enum FieldInfo { - Positional(usize), - Named(BTreeSet), - Empty, +pub struct ResolvedBuiltinFunction { + pub fun: N::BuiltinFunction, } #[derive(Debug, Clone)] -pub(super) enum ResolvedConstructor { - Struct(DatatypeName, Box), - Variant( - Box, - VariantName, - /* variant decl loc */ Loc, - Box, - ), +pub enum ResolvedDatatype { + Struct(Box), + Enum(Box), } -enum ResolvedFunction { - Builtin(N::BuiltinFunction), - Module(Box), - Var(N::Var), +#[derive(Debug, Clone)] +pub enum ResolvedModuleMember { + Datatype(ResolvedDatatype), + Function(Box), + Constant(Box), +} + +// ------------------------------------------------------------------------------------------------- +// Resolution Result Types +// ------------------------------------------------------------------------------------------------- +// These type definitions are the result of a resolution call, based on the type of thing you are +// trying to resolve. + +#[derive(Debug, Clone)] +pub(super) enum ResolvedType { + ModuleType(ResolvedDatatype), + TParam(Loc, N::TParam), + BuiltinType(N::BuiltinTypeName_), + Hole, // '_' type Unbound, } -struct ResolvedModuleFunction { - // original names/locs are provided to preserve loc information if needed - module: ModuleIdent, - function: FunctionName, - ty_args: Option>, +#[derive(Debug, Clone)] +pub(super) enum ResolvedConstructor { + Struct(Box), + Variant(Box), } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ResolveFunctionCase { - UseFun, - Call, +#[derive(Debug, Clone)] +pub(super) enum ResolvedCallSubject { + Builtin(Box), + Constructor(Box), + Function(Box), + Var(Box), + Unbound, } -enum ResolvedModuleAccess { - Function(FunctionName), - Constant(ConstantName), - Datatype(ModuleType), +#[derive(Debug, Clone)] +pub(super) enum ResolvedUseFunFunction { + Builtin(Box), + Module(Box), + Unbound, } -enum ModuleAccessKind { - Function, - Datatype, - Constant, +#[derive(Debug, Clone)] +pub(super) enum ResolvedTerm { + Constant(Box), + Variant(Box), + Var(Box), + Unbound, } +#[derive(Debug, Clone)] +pub(super) enum ResolvedPatternTerm { + Constant(Box), + Constructor(Box), + Unbound, +} + +// ------------------------------------------------------------------------------------------------- +// Block Types +// ------------------------------------------------------------------------------------------------- + #[derive(PartialEq, Eq, Copy, Clone, Debug)] enum LoopType { While, @@ -145,6 +182,11 @@ enum NominalBlockType { LambdaLoopCapture, } +// ------------------------------------------------------------------------------------------------- +// Resolution Flags +// ------------------------------------------------------------------------------------------------- +// These are for determining what's gong on during resoluiton. + #[derive(PartialEq, Eq, Copy, Clone, Debug)] enum TypeAnnotation { StructField, @@ -155,71 +197,75 @@ enum TypeAnnotation { Expression, } +//************************************************ +// impls +//************************************************ + impl ResolvedType { - fn is_struct(&self) -> bool { + /// Set the information for the module identifier and name to the ones provided. This allows + /// name resolution to preserve location information and address names from the original name + /// onto the resolved one. + #[allow(dead_code)] + fn set_locs(&mut self, mident: ModuleIdent, name_loc: Loc) { match self { - ResolvedType::ModuleType(rt) => match rt.module_type { - ModuleType::Struct(..) => true, - ModuleType::Enum(..) => false, - }, - _ => false, + ResolvedType::ModuleType(mtype) => mtype.set_name_info(mident, name_loc), + ResolvedType::TParam(loc, _) => *loc = name_loc, + ResolvedType::BuiltinType(_) => (), + ResolvedType::Hole => (), + ResolvedType::Unbound => (), } } +} - #[allow(dead_code)] - fn is_enum(&self) -> bool { +impl ResolvedDatatype { + fn decl_loc(&self) -> Loc { match self { - ResolvedType::ModuleType(rt) => match rt.module_type { - ModuleType::Struct(..) => false, - ModuleType::Enum(..) => true, - }, - _ => false, + ResolvedDatatype::Struct(stype) => stype.decl_loc, + ResolvedDatatype::Enum(etype) => etype.decl_loc, } } -} - -//************************************************ -// impls -//************************************************ -impl ModuleType { - fn decl_loc(&self) -> Loc { + fn mident(&self) -> ModuleIdent { match self { - ModuleType::Struct(stype) => stype.decl_loc, - ModuleType::Enum(etype) => etype.decl_loc, + ResolvedDatatype::Struct(stype) => stype.mident, + ResolvedDatatype::Enum(etype) => etype.mident, } } - fn original_mident(&self) -> ModuleIdent { + fn name(&self) -> DatatypeName { match self { - ModuleType::Struct(stype) => stype.original_mident, - ModuleType::Enum(etype) => etype.original_mident, + ResolvedDatatype::Struct(stype) => stype.name, + ResolvedDatatype::Enum(etype) => etype.name, } } - fn with_original_mident(self, mident: ModuleIdent) -> ModuleType { + fn name_symbol(&self) -> Symbol { match self { - ModuleType::Struct(stype) => { - let st = StructType { - original_mident: mident, - ..*stype - }; - ModuleType::Struct(Box::new(st)) - } - ModuleType::Enum(etype) => { - let et = EnumType { - original_mident: mident, - ..*etype - }; - ModuleType::Enum(Box::new(et)) - } + ResolvedDatatype::Struct(stype) => stype.name.value(), + ResolvedDatatype::Enum(etype) => etype.name.value(), } } fn datatype_kind_str(&self) -> String { match self { - ModuleType::Struct(_) => "struct".to_string(), - ModuleType::Enum(_) => "enum".to_string(), + ResolvedDatatype::Struct(_) => "struct".to_string(), + ResolvedDatatype::Enum(_) => "enum".to_string(), + } + } + + /// Set the information for the module identifier and name to the ones provided. This allows + /// name resolution to preserve location information and address names from the original name + /// onto the resolved one. + fn set_name_info(&mut self, mident: ModuleIdent, name_loc: Loc) { + match self { + ResolvedDatatype::Struct(stype) => { + stype.mident = mident; + stype.name = stype.name.with_loc(name_loc); + } + ResolvedDatatype::Enum(etype) => { + etype.mident = mident; + etype.name = etype.name.with_loc(name_loc); + } } } } @@ -245,44 +291,217 @@ impl FieldInfo { impl ResolvedConstructor { fn type_arity(&self) -> usize { match self { - ResolvedConstructor::Struct(_, stype) => stype.arity, - ResolvedConstructor::Variant(etype, _, _, _) => etype.arity, + ResolvedConstructor::Struct(stype) => stype.tyarg_arity, + ResolvedConstructor::Variant(vtype) => vtype.tyarg_arity, } } fn field_info(&self) -> &FieldInfo { match self { - ResolvedConstructor::Struct(_, stype) => &stype.field_info, - ResolvedConstructor::Variant(_, _, _, field_info) => field_info, + ResolvedConstructor::Struct(stype) => &stype.field_info, + ResolvedConstructor::Variant(vtype) => &vtype.field_info, } } - fn name(&self) -> String { + fn type_name(&self) -> String { match self { - ResolvedConstructor::Struct(name, _) => name.to_string(), - ResolvedConstructor::Variant(_, vname, _, _) => vname.to_string(), + ResolvedConstructor::Struct(s) => format!("{}::{}", s.mident, s.name), + ResolvedConstructor::Variant(v) => format!("{}::{}", v.mident, v.enum_name), + } + } + + fn name_symbol(&self) -> Symbol { + match self { + ResolvedConstructor::Struct(stype) => stype.name.value(), + ResolvedConstructor::Variant(vtype) => vtype.name.value(), } } } -impl std::fmt::Display for ResolvedModuleAccess { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl ResolvedVariant { + /// Set the information for the module identifier, enum_name, and name to the ones provided. + /// This allows name resolution to preserve location information and address names from the + /// original name onto the resolved one. + fn set_name_info(&mut self, mident: ModuleIdent, enum_name_loc: Loc, name_loc: Loc) { + self.mident = mident; + self.enum_name = self.enum_name.with_loc(enum_name_loc); + self.name = self.name.with_loc(name_loc); + } +} + +impl ResolvedConstant { + /// Set the information for the module identifier and name to the ones provided. This allows + /// name resolution to preserve location information and address names from the original name + /// onto the resolved one. + fn set_name_info(&mut self, mident: ModuleIdent, name_loc: Loc) { + self.mident = mident; + self.name = self.name.with_loc(name_loc); + } +} + +impl ResolvedModuleFunction { + /// Set the information for the module identifier and name to the ones provided. This allows + /// name resolution to preserve location information and address names from the original name + /// onto the resolved one. + fn set_name_info(&mut self, mident: ModuleIdent, name_loc: Loc) { + self.mident = mident; + self.name = self.name.with_loc(name_loc); + } +} + +impl ResolvedModuleMember { + /// Set the information for the module identifier and name to the ones provided. This allows + /// name resolution to preserve location information and address names from the original name + /// onto the resolved one. + fn set_name_info(&mut self, mident: ModuleIdent, name_loc: Loc) { + match self { + ResolvedModuleMember::Datatype(member) => member.set_name_info(mident, name_loc), + ResolvedModuleMember::Function(member) => member.set_name_info(mident, name_loc), + ResolvedModuleMember::Constant(member) => member.set_name_info(mident, name_loc), + } + } + + fn mident(&self) -> ModuleIdent { match self { - ResolvedModuleAccess::Function(_) => write!(f, "function"), - ResolvedModuleAccess::Constant(_) => write!(f, "constant"), - ResolvedModuleAccess::Datatype(ty) => write!(f, "{}", ty.datatype_kind_str()), + ResolvedModuleMember::Datatype(dt) => dt.mident(), + ResolvedModuleMember::Function(fun) => fun.mident, + ResolvedModuleMember::Constant(const_) => const_.mident, + } + } + + fn name_symbol(&self) -> Symbol { + match self { + ResolvedModuleMember::Datatype(dt) => dt.name_symbol(), + ResolvedModuleMember::Function(fun) => fun.name.value(), + ResolvedModuleMember::Constant(const_) => const_.name.value(), } } } -impl std::fmt::Display for ModuleAccessKind { +impl std::fmt::Display for ResolvedModuleMember { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - ModuleAccessKind::Function => write!(f, "function"), - ModuleAccessKind::Constant => write!(f, "constant"), - ModuleAccessKind::Datatype => write!(f, "type"), + ResolvedModuleMember::Function(_) => write!(f, "function"), + ResolvedModuleMember::Constant(_) => write!(f, "constant"), + ResolvedModuleMember::Datatype(ty) => write!(f, "{}", ty.datatype_kind_str()), + } + } +} + +//************************************************************************************************** +// Module Index +//************************************************************************************************** +// This indes is used for looking full paths up in name resolution. + +pub type ModuleMembers = BTreeMap>; + +pub fn build_member_map( + env: &CompilationEnv, + pre_compiled_lib: Option>, + prog: &E::Program, +) -> ModuleMembers { + // NB: This checks if the element is present, and doesn't replace it if so. This is congruent + // with how top-level definitions are handled for alias resolution, where a new definition will + // not overwrite the previous one. + macro_rules! add_or_error { + ($members:ident, $name:expr, $value:expr) => {{ + let name = $name.value(); + if $members.contains_key(&name) { + assert!(env.has_errors()); + } else { + $members.insert(name, $value); + } + }}; + } + + use ResolvedModuleMember as M; + let all_modules = prog + .modules + .key_cloned_iter() + .chain(pre_compiled_lib.iter().flat_map(|pre_compiled| { + pre_compiled + .expansion + .modules + .key_cloned_iter() + .filter(|(mident, _m)| !prog.modules.contains_key(mident)) + })); + let mut all_members = BTreeMap::new(); + for (mident, mdef) in all_modules { + let mut members = BTreeMap::new(); + for (name, sdef) in mdef.structs.key_cloned_iter() { + let tyarg_arity = sdef.type_parameters.len(); + let field_info = match &sdef.fields { + E::StructFields::Positional(fields) => FieldInfo::Positional(fields.len()), + E::StructFields::Named(f) => { + FieldInfo::Named(f.key_cloned_iter().map(|(k, _)| k).collect()) + } + E::StructFields::Native(_) => FieldInfo::Empty, + }; + let struct_def = ResolvedStruct { + mident, + name, + decl_loc: name.loc(), + tyarg_arity, + field_info, + }; + assert!(members.insert(name.value(), M::Datatype(ResolvedDatatype::Struct(Box::new(struct_def)))).is_none()) } + for (enum_name, edef) in mdef.enums.key_cloned_iter() { + let tyarg_arity = edef.type_parameters.len(); + let variants = edef.variants.clone().map(|name, v| { + let field_info = match &v.fields { + E::VariantFields::Named(fields) => { + FieldInfo::Named(fields.key_cloned_iter().map(|(k, _)| k).collect()) + } + E::VariantFields::Positional(tys) => FieldInfo::Positional(tys.len()), + E::VariantFields::Empty => FieldInfo::Empty, + }; + ResolvedVariant { + mident, + enum_name, + tyarg_arity, + name, + decl_loc: v.loc, + field_info, + } + }); + let decl_loc = edef.loc; + let enum_def = ResolvedEnum { + mident, + name: enum_name, + decl_loc, + tyarg_arity, + variants, + }; + add_or_error!( + members, + enum_name, + M::Datatype(ResolvedDatatype::Enum(Box::new(enum_def))) + ); + } + // Functions and constants are shadowed by datatypes that share their names. + for (name, fun) in mdef.functions.key_cloned_iter() { + let tyarg_arity = fun.signature.type_parameters.len(); + let arity = fun.signature.parameters.len(); + let fun_def = ResolvedModuleFunction { + mident, + name, + tyarg_arity, + arity, + }; + add_or_error!(members, name, M::Function(Box::new(fun_def))); + } + for (name, _) in mdef.constants.key_cloned_iter() { + let const_def = ResolvedConstant { + mident, + name, + decl_loc: name.loc(), + }; + add_or_error!(members, name, M::Constant(Box::new(const_def))); + } + assert!(all_members.insert(mident, members).is_none()); } + all_members } //************************************************************************************************** @@ -292,11 +511,10 @@ impl std::fmt::Display for ModuleAccessKind { pub(super) struct Context<'env> { pub env: &'env mut CompilationEnv, current_module: Option, - scoped_types: BTreeMap>, + /// Nothing should ever use this directly, and should instead go through + /// `resolve_module_access` because it preserves source location information. + module_members: ModuleMembers, unscoped_types: Vec>, - scoped_functions: BTreeMap>, - scoped_constants: BTreeMap>, - modules: BTreeSet, local_scopes: Vec>, local_count: BTreeMap, used_locals: BTreeSet, @@ -312,34 +530,16 @@ pub(super) struct Context<'env> { macro_rules! resolve_from_module_access { ($context:expr, $loc:expr, $mident:expr, $name:expr, $expected_pat:pat, $rhs:expr, $expected_kind:expr) => {{ - if !$context.modules.contains($mident) { - $context.env.add_diag(diag!( - NameResolution::UnboundModule, - ($mident.loc, format!("Unbound module '{}'", $mident)), - )); - return None; - } - match $context.resolve_module_access($mident, $name, $expected_kind) { + match $context.resolve_module_access(&Some($expected_kind), $loc, $mident, $name) { Some($expected_pat) => $rhs, Some(other) => { - let msg = format!( - "Invalid module access. \ - Expected a {}, but found {} '{}' in module '{}'", - $expected_kind, other, $name, $mident - ); - $context - .env - .add_diag(diag!(NameResolution::UnboundModuleMember, ($loc, msg))); + let diag = + make_invalid_module_member_kind_error($context, &$expected_kind, $loc, &other); + $context.env.add_diag(diag); None } None => { - let msg = format!( - "Invalid module access. Unbound {} '{}' in module '{}'", - $expected_kind, $name, $mident - ); - $context - .env - .add_diag(diag!(NameResolution::UnboundModuleMember, ($loc, msg))); + assert!($context.env.has_errors()); None } } @@ -353,105 +553,7 @@ impl<'env> Context<'env> { prog: &E::Program, ) -> Self { use ResolvedType as RT; - let all_modules = || { - prog.modules - .key_cloned_iter() - .chain(pre_compiled_lib.iter().flat_map(|pre_compiled| { - pre_compiled - .expansion - .modules - .key_cloned_iter() - .filter(|(mident, _m)| !prog.modules.contains_key(mident)) - })) - }; - let modules = all_modules().map(|(mident, _)| mident).collect(); - let scoped_types = all_modules() - .map(|(mident, mdef)| { - let mems = { - let mut smems = mdef - .structs - .key_cloned_iter() - .map(|(s, sdef)| { - let arity = sdef.type_parameters.len(); - let sname = s.value(); - let field_info = match &sdef.fields { - E::StructFields::Positional(fields) => { - FieldInfo::Positional(fields.len()) - } - E::StructFields::Named(f) => { - FieldInfo::Named(f.key_cloned_iter().map(|(k, _)| k).collect()) - } - E::StructFields::Native(_) => FieldInfo::Empty, - }; - let st = StructType { - original_mident: mident, - decl_loc: s.loc(), - arity, - field_info, - }; - let type_info = ModuleType::Struct(Box::new(st)); - (sname, type_info) - }) - .collect::>(); - let mut emems = mdef - .enums - .key_cloned_iter() - .map(|(e, edef)| { - let arity = edef.type_parameters.len(); - let ename = e.value(); - let variants = edef.variants.clone().map(|name, v| { - let field_info = match &v.fields { - E::VariantFields::Named(fields) => FieldInfo::Named( - fields.key_cloned_iter().map(|(k, _)| k).collect(), - ), - E::VariantFields::Positional(tys) => { - FieldInfo::Positional(tys.len()) - } - E::VariantFields::Empty => FieldInfo::Empty, - }; - VariantConstructor { - original_variant_name: name.0, - decl_loc: v.loc, - field_info, - } - }); - let et = EnumType { - original_mident: mident, - arity, - decl_loc: e.loc(), - variants, - }; - let type_info = ModuleType::Enum(Box::new(et)); - (ename, type_info) - }) - .collect::>(); - /* duplicates were already reported by expasion */ - smems.append(&mut emems); - smems - }; - (mident, mems) - }) - .collect(); - let scoped_functions = all_modules() - .map(|(mident, mdef)| { - let mems = mdef - .functions - .iter() - .map(|(nloc, n, _)| (*n, nloc)) - .collect(); - (mident, mems) - }) - .collect(); - let scoped_constants = all_modules() - .map(|(mident, mdef)| { - let mems = mdef - .constants - .iter() - .map(|(nloc, n, _)| (*n, nloc)) - .collect(); - (mident, mems) - }) - .collect(); + let module_members = build_member_map(compilation_env, pre_compiled_lib, prog); let unscoped_types = vec![N::BuiltinTypeName_::all_names() .iter() .map(|s| { @@ -462,10 +564,7 @@ impl<'env> Context<'env> { Self { env: compilation_env, current_module: None, - modules, - scoped_types, - scoped_functions, - scoped_constants, + module_members, unscoped_types, local_scopes: vec![], local_count: BTreeMap::new(), @@ -478,92 +577,98 @@ impl<'env> Context<'env> { } } - fn resolve_module(&mut self, m: &ModuleIdent) -> bool { - // NOTE: piggybacking on `scoped_functions` to provide a set of modules in the context。 - // TODO: a better solution would be to have a single `BTreeMap` - // in the context that can be used to resolve modules, types, and functions. - let resolved = self.scoped_functions.contains_key(m); + fn valid_module(&mut self, m: &ModuleIdent) -> bool { + let resolved = self.module_members.contains_key(m); if !resolved { - self.env.add_diag(diag!( - NameResolution::UnboundModule, - (m.loc, format!("Unbound module '{}'", m)) - )) + let diag = make_unbound_module_error(self, m.loc, m); + self.env.add_diag(diag); } resolved } - fn resolve_module_type_opt(&self, m: &ModuleIdent, n: &Name) -> Option { - if let Some(types) = self.scoped_types.get(m) { - types.get(&n.value).cloned() - } else { - None + /// Main module access resolver. Everything for modules should go through this when possible, + /// as it automatically preserves location information on symbols. + fn resolve_module_access( + &mut self, + kind: &Option, + loc: Loc, + m: &ModuleIdent, + n: &Name, + ) -> Option { + let Some(members) = self.module_members.get(m) else { + self.env.add_diag(make_unbound_module_error(self, m.loc, m)); + return None; + }; + let result = members.get(&n.value); + if result.is_none() { + let diag = make_unbound_module_member_error(self, kind, loc, *m, n.value); + self.env.add_diag(diag); } + result.map(|inner| { + let mut result = inner.clone(); + result.set_name_info(*m, n.loc); + result + }) } - fn resolve_function_opt(&self, m: &ModuleIdent, n: &Name) -> Option { - self.scoped_functions - .get(m) - .and_then(|functions| functions.get(&n.value).map(|_| FunctionName(*n))) - } - - fn resolve_constant_opt(&self, m: &ModuleIdent, n: &Name) -> Option { - self.scoped_constants - .get(m) - .and_then(|constants| constants.get(&n.value).map(|_| ConstantName(*n))) + fn resolve_module_type( + &mut self, + loc: Loc, + m: &ModuleIdent, + n: &Name, + error_kind: ErrorKind, + ) -> Option> { + resolve_from_module_access!( + self, + loc, + m, + n, + ResolvedModuleMember::Datatype(module_type), + Some(Box::new(module_type)), + error_kind + ) } - fn resolve_module_access( + fn resolve_module_function( &mut self, + loc: Loc, m: &ModuleIdent, n: &Name, - expected_access_type: ModuleAccessKind, - ) -> Option { - let function_opt = self - .resolve_function_opt(m, n) - .map(ResolvedModuleAccess::Function); - let constant_opt = self - .resolve_constant_opt(m, n) - .map(ResolvedModuleAccess::Constant); - let datatype_opt = self - .resolve_module_type_opt(m, n) - .map(ResolvedModuleAccess::Datatype); - match expected_access_type { - ModuleAccessKind::Function => function_opt.or(datatype_opt).or(constant_opt), - ModuleAccessKind::Constant => constant_opt.or(datatype_opt).or(function_opt), - ModuleAccessKind::Datatype => datatype_opt.or(constant_opt).or(function_opt), - } - } - - fn resolve_module_type(&mut self, loc: Loc, m: &ModuleIdent, n: &Name) -> Option { + ) -> Option> { resolve_from_module_access!( self, loc, m, n, - ResolvedModuleAccess::Datatype(module_type), - Some(module_type), - ModuleAccessKind::Datatype + ResolvedModuleMember::Function(fun), + Some(fun), + ErrorKind::Function ) } - fn resolve_module_function( + #[allow(dead_code)] + fn resolve_module_constant( &mut self, loc: Loc, m: &ModuleIdent, n: &Name, - ) -> Option { + ) -> Option> { resolve_from_module_access!( self, loc, m, n, - ResolvedModuleAccess::Function(name), - Some(name), - ModuleAccessKind::Function + ResolvedModuleMember::Constant(const_), + Some(const_), + ErrorKind::Constant ) } - pub fn resolve_type(&mut self, sp!(nloc, ma_): E::ModuleAccess) -> ResolvedType { + fn resolve_type_inner( + &mut self, + sp!(nloc, ma_): E::ModuleAccess, + error_kind: ErrorKind, + ) -> ResolvedType { use E::ModuleAccess_ as EN; match ma_ { EN::Name(sp!(_, n)) if n == symbol!("_") => { @@ -572,22 +677,34 @@ impl<'env> Context<'env> { .check_feature(current_package, FeatureGate::TypeHoles, nloc); ResolvedType::Hole } - EN::Name(n) => self.resolve_unscoped_type(nloc, n), + EN::Name(n) => match self.resolve_unscoped_type(nloc, n) { + ResolvedType::ModuleType(mut module_type) => { + module_type.set_name_info(self.current_module.unwrap(), nloc); + ResolvedType::ModuleType(module_type) + } + ty @ (ResolvedType::BuiltinType(_) + | ResolvedType::TParam(_, _) + | ResolvedType::Hole + | ResolvedType::Unbound) => ty, + }, EN::ModuleAccess(m, n) | EN::Variant(sp!(_, (m, n)), _) => { - let Some(module_type) = self.resolve_module_type(nloc, &m, &n) else { + let Some(module_type) = self.resolve_module_type(nloc, &m, &n, error_kind) else { assert!(self.env.has_errors()); return ResolvedType::Unbound; }; - let mt = ResolvedModuleType { - original_loc: nloc, - original_type_name: n, - module_type: module_type.with_original_mident(m), - }; - ResolvedType::ModuleType(Box::new(mt)) + ResolvedType::ModuleType(*module_type) } } } + pub fn resolve_type(&mut self, access: E::ModuleAccess) -> ResolvedType { + self.resolve_type_inner(access, ErrorKind::Type) + } + + pub fn resolve_type_for_constructor(&mut self, access: E::ModuleAccess) -> ResolvedType { + self.resolve_type_inner(access, ErrorKind::Datatype) + } + fn resolve_unscoped_type(&mut self, loc: Loc, n: Name) -> ResolvedType { match self .unscoped_types @@ -596,22 +713,161 @@ impl<'env> Context<'env> { .find_map(|unscoped_types| unscoped_types.get(&n.value)) { None => { - let msg = format!("Unbound type '{}' in current scope", n); - self.env - .add_diag(diag!(NameResolution::UnboundType, (loc, msg))); + let diag = make_unbound_local_name_error(self, &ErrorKind::Type, loc, n); + self.env.add_diag(diag); ResolvedType::Unbound } Some(rn) => rn.clone(), } } + fn resolve_call_subject(&mut self, sp!(mloc, ma_): E::ModuleAccess) -> ResolvedCallSubject { + use ErrorKind as EK; + use E::ModuleAccess_ as EA; + use N::BuiltinFunction_ as B; + match ma_ { + EA::ModuleAccess(m, n) => { + match self.resolve_module_access(&Some(ErrorKind::Function), mloc, &m, &n) { + Some(ResolvedModuleMember::Function(fun)) => ResolvedCallSubject::Function(fun), + Some(ResolvedModuleMember::Datatype(ResolvedDatatype::Struct(struct_))) => { + ResolvedCallSubject::Constructor(Box::new(ResolvedConstructor::Struct( + struct_, + ))) + } + Some(c @ ResolvedModuleMember::Constant(_)) => { + let diag = + make_invalid_module_member_kind_error(self, &EK::Function, mloc, &c); + self.env.add_diag(diag); + ResolvedCallSubject::Unbound + } + Some(e @ ResolvedModuleMember::Datatype(ResolvedDatatype::Enum(_))) => { + let mut diag = + make_invalid_module_member_kind_error(self, &EK::Function, mloc, &e); + diag.add_note("Enums cannot be instantiated directly. Instead, you must instantiate a variant."); + self.env.add_diag(diag); + ResolvedCallSubject::Unbound + } + None => { + assert!(self.env.has_errors()); + ResolvedCallSubject::Unbound + } + } + } + EA::Name(n) if N::BuiltinFunction_::all_names().contains(&n.value) => { + let fun_ = match n.value.as_str() { + B::FREEZE => B::Freeze(None), + B::ASSERT_MACRO => { + B::Assert(/* is_macro, set by caller */ None) + } + _ => { + let diag = + make_unbound_local_name_error(self, &EK::Function, n.loc, n.value); + self.env.add_diag(diag); + return ResolvedCallSubject::Unbound; + } + }; + let fun = sp(mloc, fun_); + let resolved = ResolvedBuiltinFunction { fun }; + ResolvedCallSubject::Builtin(Box::new(resolved)) + } + EA::Name(n) => { + match self.resolve_local( + n.loc, + NameResolution::UnboundUnscopedName, + |n| format!("Unbound function '{}' in current scope", n), + n, + ) { + None => { + assert!(self.env.has_errors()); + ResolvedCallSubject::Unbound + } + Some(v) => ResolvedCallSubject::Var(Box::new(sp(n.loc, v.value))), + } + } + EA::Variant(inner, _) => { + let sloc = inner.loc; + match self.resolve_datatype_constructor(sp(mloc, ma_), "construction") { + Some(variant @ ResolvedConstructor::Variant(_)) => { + ResolvedCallSubject::Constructor(Box::new(variant)) + } + Some(ResolvedConstructor::Struct(struct_)) => { + self.env.add_diag(diag!( + NameResolution::NamePositionMismatch, + (sloc, "Invalid constructor. Expected an enum".to_string()), + ( + struct_.decl_loc, + format!("But '{}' is an struct", struct_.name) + ) + )); + ResolvedCallSubject::Unbound + } + None => { + assert!(self.env.has_errors()); + ResolvedCallSubject::Unbound + } + } + } + } + } + + fn resolve_use_fun_function( + &mut self, + sp!(mloc, ma_): E::ModuleAccess, + ) -> ResolvedUseFunFunction { + use E::ModuleAccess_ as EA; + use N::BuiltinFunction_ as B; + match ma_ { + EA::ModuleAccess(m, n) => match self.resolve_module_function(mloc, &m, &n) { + None => { + assert!(self.env.has_errors()); + ResolvedUseFunFunction::Unbound + } + Some(mut fun) => { + // Change the names to have the correct locations + fun.mident.loc = m.loc; + fun.name = fun.name.with_loc(n.loc); + ResolvedUseFunFunction::Module(fun) + } + }, + EA::Name(n) if N::BuiltinFunction_::all_names().contains(&n.value) => { + let fun_ = match n.value.as_str() { + B::FREEZE => B::Freeze(None), + B::ASSERT_MACRO => { + B::Assert(/* is_macro, set by caller */ None) + } + _ => { + let diag = + make_unbound_local_name_error(self, &ErrorKind::Function, n.loc, n); + self.env.add_diag(diag); + return ResolvedUseFunFunction::Unbound; + } + }; + let fun = sp(mloc, fun_); + let resolved = ResolvedBuiltinFunction { fun }; + ResolvedUseFunFunction::Builtin(Box::new(resolved)) + } + EA::Name(n) => { + let diag = make_unbound_local_name_error(self, &ErrorKind::Function, n.loc, n); + self.env.add_diag(diag); + ResolvedUseFunFunction::Unbound + } + EA::Variant(_, _) => { + self.env.add_diag(ice!(( + mloc, + "Tried to resolve variant '{}' as a function in current scope" + ),)); + ResolvedUseFunFunction::Unbound + } + } + } + fn resolve_datatype_constructor( &mut self, ma: E::ModuleAccess, verb: &str, - ) -> Option<(ModuleIdent, DatatypeName, ResolvedConstructor)> { + ) -> Option { use E::ModuleAccess_ as EN; - match self.resolve_type(ma) { + match self.resolve_type_for_constructor(ma) { ResolvedType::Unbound => { assert!(self.env.has_errors()); None @@ -651,60 +907,50 @@ impl<'env> Context<'env> { )); None } - ResolvedType::ModuleType(mt) => { - let ResolvedModuleType { - module_type, - original_type_name: name, - .. - } = *mt; - let mident_name = module_type.original_mident(); + ResolvedType::ModuleType(module_type) => { + use ResolvedDatatype as D; match (&ma.value, module_type) { - (EN::Name(_) | EN::ModuleAccess(_, _), ModuleType::Struct(struct_type)) => { - let dname = DatatypeName(name); - Some(( - mident_name, - dname, - ResolvedConstructor::Struct(dname, struct_type), - )) + (EN::Name(_) | EN::ModuleAccess(_, _), D::Struct(struct_type)) => { + Some(ResolvedConstructor::Struct(struct_type)) } - (EN::Variant(_, variant_name), ModuleType::Enum(enum_type)) => { + (EN::Variant(_, variant_name), D::Enum(enum_type)) => { let vname = VariantName(*variant_name); - let Some(variant_info) = enum_type.variants.get(&vname).cloned() else { + let Some(mut variant_info) = enum_type.variants.get(&vname).cloned() else { let primary_msg = format!( "Invalid {verb}. Variant '{variant_name}' is not part of this enum", ); - let decl_msg = format!("Enum '{name}' is defined here"); + let decl_msg = format!("Enum '{}' is defined here", enum_type.name); self.env.add_diag(diag!( NameResolution::UnboundVariant, (ma.loc, primary_msg), - (name.loc, decl_msg), + (enum_type.decl_loc, decl_msg), )); return None; }; - Some(( - mident_name, - DatatypeName(name), - ResolvedConstructor::Variant( - enum_type, - vname, - variant_info.decl_loc, - Box::new(variant_info.field_info.clone()), - ), - )) + // The `enum_type` had its locations updated by `resolve_type`. + variant_info.set_name_info( + enum_type.mident, + enum_type.name.loc(), + variant_name.loc, + ); + Some(ResolvedConstructor::Variant(Box::new(variant_info))) } - (EN::Name(_) | EN::ModuleAccess(_, _), ModuleType::Enum(_)) => { + (EN::Name(_) | EN::ModuleAccess(_, _), D::Enum(enum_type)) => { self.env.add_diag(diag!( NameResolution::NamePositionMismatch, (ma.loc, format!("Invalid {verb}. Expected a struct")), - (name.loc, format!("But '{name}' is an enum")) + ( + enum_type.decl_loc, + format!("But '{}' is an enum", enum_type.name) + ) )); None } - (EN::Variant(sp!(sloc, _), _), ModuleType::Struct(_)) => { + (EN::Variant(sp!(sloc, _), _), D::Struct(stype)) => { self.env.add_diag(diag!( NameResolution::NamePositionMismatch, (*sloc, format!("Invalid {verb}. Expected an enum")), - (name.loc, format!("But '{name}' is a struct")) + (stype.decl_loc, format!("But '{}' is an struct", stype.name)) )); None } @@ -713,209 +959,151 @@ impl<'env> Context<'env> { } } - fn resolves_to_datatype(&self, sp!(_, ma_): &E::ModuleAccess) -> bool { - use E::ModuleAccess_ as EA; + fn resolve_term(&mut self, sp!(mloc, ma_): E::ModuleAccess) -> ResolvedTerm { match ma_ { - EA::Name(n) => self - .unscoped_types - .iter() - .rev() - .find_map(|unscoped_types| unscoped_types.get(&n.value)) - .is_some_and(|rt| rt.is_struct() || matches!(rt, ResolvedType::BuiltinType(_))), - EA::ModuleAccess(m, n) => self - .scoped_types - .get(m) - .and_then(|types| types.get(&n.value)) - .is_some(), - EA::Variant(sp!(_, (m, n)), _) => self - .scoped_types - .get(m) - .and_then(|types| types.get(&n.value)) - .is_some(), - } - } - - fn resolve_struct_name( - &mut self, - loc: Loc, - verb: &str, - ma: E::ModuleAccess, - etys_opt: Option>, - ) -> Option<(ModuleIdent, DatatypeName, Option>, FieldInfo)> { - match self.resolve_type(ma) { - ResolvedType::Unbound => { - assert!(self.env.has_errors()); - None - } - rt @ (ResolvedType::BuiltinType(_) - | ResolvedType::TParam(_, _) - | ResolvedType::Hole) => { - let (rtloc, msg) = match rt { - ResolvedType::TParam(loc, tp) => ( - loc, - format!( - "But '{}' was declared as a type parameter here", - tp.user_specified_name - ), - ), - ResolvedType::BuiltinType(n) => { - (ma.loc, format!("But '{n}' is a builtin type")) - } - ResolvedType::Hole => ( - ma.loc, - "The '_' is a placeholder for type inference".to_owned(), - ), - _ => unreachable!(), - }; - self.env.add_diag(diag!( - NameResolution::NamePositionMismatch, - (ma.loc, format!("Invalid {}. Expected a struct name", verb)), - (rtloc, msg) - )); - None - } - ResolvedType::ModuleType(mt) => { - let ResolvedModuleType { - module_type, - original_type_name: n, - .. - } = *mt; - match module_type { - ModuleType::Struct(struct_type) => { - let m = struct_type.original_mident; - let tys_opt = etys_opt.map(|etys| { - let tys = types(self, TypeAnnotation::Expression, etys); - let name_f = || format!("{}::{}", &m, &n); - check_type_argument_arity(self, loc, name_f, tys, struct_type.arity) - }); - Some((m, DatatypeName(n), tys_opt, struct_type.field_info)) + E::ModuleAccess_::Name(name) if !is_constant_name(&name.value) => { + match self.resolve_local( + mloc, + NameResolution::UnboundVariable, + |name| format!("Unbound variable '{name}'"), + name, + ) { + None => { + debug_assert!(self.env.has_errors()); + ResolvedTerm::Unbound } - ModuleType::Enum(..) => { - self.env.add_diag(diag!( - NameResolution::NamePositionMismatch, - (ma.loc, format!("Invalid {}. Expected a struct", verb)), - (n.loc, format!("But '{}' is an enum", n)) - )); - None + Some(mut nv) => { + nv.loc = mloc; + ResolvedTerm::Var(Box::new(nv)) } } } - } - } - - fn resolves_to_constant(&self, sp!(_, ma_): &E::ModuleAccess) -> bool { - use E::ModuleAccess_ as EA; - match ma_ { - EA::Name(_) => false, // constants are expanded during naming - EA::ModuleAccess(m, n) => self - .scoped_constants - .get(m) - .and_then(|constants| constants.get(&n.value)) - .is_some(), - EA::Variant(_, _) => false, - } - } - - fn resolve_constant( - &mut self, - sp!(loc, ma_): E::ModuleAccess, - ) -> Option<(ModuleIdent, ConstantName)> { - use E::ModuleAccess_ as EA; - match ma_ { - EA::Name(n) => { + E::ModuleAccess_::Name(name) => { self.env.add_diag(diag!( NameResolution::UnboundUnscopedName, - (loc, format!("Unbound constant '{}'", n)), + (mloc, format!("Unbound constant '{}'", name)), )); - None + ResolvedTerm::Unbound } - EA::ModuleAccess(m, n) => { - if !self.modules.contains(&m) { - self.env.add_diag(diag!( - NameResolution::UnboundModule, - (m.loc, format!("Unbound module '{m}'")), - )); - return None; - } - match self.resolve_module_access(&m, &n, ModuleAccessKind::Constant) { - Some(ResolvedModuleAccess::Constant(cname)) => Some((m, cname)), - Some(ResolvedModuleAccess::Datatype(module_type)) => { - match module_type { - ModuleType::Struct(stype) => { - let tyargs = arity_string(stype.arity); - let msg = format!( - "Expected local or constant, \ - found struct '{n}' in module '{m}' instead." - ); - let mut diag = diag!(NameResolution::InvalidPosition, (loc, msg)); - if stype.field_info.is_positional() { - diag.add_note(format!( - "Structs with positional arguments must be written as \ - '{n}{tyargs}( ... )'" - )); - } else { - diag.add_note(format!( - "Struct with named arguments must be written as \ - '{n}{tyargs} {{ ... }}'" - )); + E::ModuleAccess_::ModuleAccess(m, n) => { + match self.resolve_module_access(&Some(ErrorKind::ModuleMember), mloc, &m, &n) { + Some(entry) => match entry { + ResolvedModuleMember::Constant(const_) => ResolvedTerm::Constant(const_), + r @ (ResolvedModuleMember::Datatype(_) + | ResolvedModuleMember::Function(_)) => { + let mut diag = make_invalid_module_member_kind_error( + self, + &ErrorKind::Constant, + mloc, + &r, + ); + match r { + ResolvedModuleMember::Datatype(ResolvedDatatype::Enum(etype)) => { + let arity = arity_string(etype.tyarg_arity); + if let Some((_, vname, ctor)) = etype.variants.iter().next() { + if ctor.field_info.is_empty() { + diag.add_note(format!( + "Enum variants with no arguments must be \ + written as '{n}::{vname}{arity}'" + )); + } else if ctor.field_info.is_positional() { + diag.add_note(format!( + "Enum variants with positional arguments must be \ + written as '{n}::{vname}{arity}( ... )'" + )); + } else { + diag.add_note(format!( + "Enum variants with named arguments must be \ + written as '{n}::{vname}{arity} {{ ... }}'" + )); + } + } } - self.env.add_diag(diag); - } - ModuleType::Enum(etype) => { - let tyargs = arity_string(etype.arity); - let msg = format!( - "Expected local or constant, \ - found enum '{n}' in module '{m}' instead." - ); - let mut diag = - diag!(NameResolution::UnboundModuleMember, (loc, msg)); - if let Some((_, vname, ctor)) = etype.variants.iter().next() { - if ctor.field_info.is_empty() { - diag.add_note(format!( - "Enum variants with no arguments must be \ - written as '{n}::{vname}{tyargs}'" - )); - } else if ctor.field_info.is_positional() { + ResolvedModuleMember::Datatype(ResolvedDatatype::Struct(stype)) => { + let arity = arity_string(stype.tyarg_arity); + if stype.field_info.is_positional() { diag.add_note(format!( - "Enum variants with positional arguments must be \ - written as '{n}::{vname}( ... ){tyargs}'" + "Structs with positional arguments must be written as \ + '{n}{arity}( ... )'" )); } else { diag.add_note(format!( - "Enum variants with named arguments must be \ - written as '{n}::{vname}{tyargs} {{ ... }}'" + "Struct with named arguments must be written as \ + '{n}{arity} {{ ... }}'" )); } - self.env.add_diag(diag); } - } + ResolvedModuleMember::Function(fun) => { + let arity = arity_string(fun.tyarg_arity); + diag.add_note(format!( + "Functions should be called as '{n}{arity}( ... )'" + )); + } + ResolvedModuleMember::Constant(_) => (), + }; + self.env.add_diag(diag); + ResolvedTerm::Unbound } - None + }, + None => { + assert!(self.env.has_errors()); + ResolvedTerm::Unbound } - Some(ResolvedModuleAccess::Function(_)) => { - let msg = format!( - "Expected local or constant, \ - found function '{n}' in module '{m}' instead." - ); - let mut diag = diag!(NameResolution::UnboundModuleMember, (loc, msg)); - diag.add_note("Functions should be called as '{n}( ... )'"); - self.env.add_diag(diag); - None + } + } + ma_ @ E::ModuleAccess_::Variant(_, _) => { + self.env + .check_feature(self.current_package, FeatureGate::Enums, mloc); + let Some(result) = self.resolve_datatype_constructor(sp(mloc, ma_), "construction") + else { + assert!(self.env.has_errors()); + return ResolvedTerm::Unbound; + }; + match result { + // TODO: this could be handed back to endure typing, similar to patterns below. + ResolvedConstructor::Struct(_) => { + assert!(self.env.has_errors()); + ResolvedTerm::Unbound } - None => { - let msg = format!( - "Invalid module access. Unbound constant '{n}' in module '{m}'" - ); - self.env - .add_diag(diag!(NameResolution::UnboundModuleMember, (loc, msg))); - None + ResolvedConstructor::Variant(variant) => ResolvedTerm::Variant(variant), + } + } + } + } + + fn resolve_pattern_term(&mut self, sp!(mloc, ma_): E::ModuleAccess) -> ResolvedPatternTerm { + match ma_ { + E::ModuleAccess_::Name(name) if !is_constant_name(&name.value) => { + self.env + .add_diag(ice!((mloc, "This should have become a binder"))); + ResolvedPatternTerm::Unbound + } + // If we have a name, try to resolve it in our module. + E::ModuleAccess_::Name(name) => { + let mut mident = self.current_module.unwrap(); + mident.loc = mloc; + let maccess = sp(mloc, E::ModuleAccess_::ModuleAccess(mident, name)); + self.resolve_pattern_term(maccess) + } + E::ModuleAccess_::ModuleAccess(m, n) => { + match self.resolve_module_access(&Some(ErrorKind::PatternTerm), mloc, &m, &n) { + // carve out constants + Some(ResolvedModuleMember::Constant(const_)) => { + ResolvedPatternTerm::Constant(const_) } + _ => match self.resolve_datatype_constructor(sp(mloc, ma_), "pattern") { + Some(ctor) => ResolvedPatternTerm::Constructor(Box::new(ctor)), + None => todo!(), + }, } } - EA::Variant(_, _) => { - self.env - .add_diag(ice!((loc, "Treated variant as a constant during naming"))); - None + ma_ @ E::ModuleAccess_::Variant(_, _) => { + let Some(ctor) = self.resolve_datatype_constructor(sp(mloc, ma_), "construction") + else { + assert!(self.env.has_errors()); + return ResolvedPatternTerm::Unbound; + }; + ResolvedPatternTerm::Constructor(Box::new(ctor)) } } } @@ -1195,6 +1383,190 @@ impl std::fmt::Display for NominalBlockType { } } +//************************************************************************************************** +// Error Reporting +//************************************************************************************************** +// TODO: use this through more of the file when possible. + +#[allow(dead_code)] +#[derive(Debug, Clone, Copy)] +enum ErrorKind { + Type, + Constructor, + Constant, + Function, + Term, + Variable, + Module, + ModuleMember, + ApplyNamed, + PatternTerm, + Datatype, +} + +impl ErrorKind { + fn kind_name(&self, context: &Context, single_name: bool) -> &str { + match self { + ErrorKind::Type => "type", + ErrorKind::Constructor + if !context + .env + .supports_feature(context.current_package, FeatureGate::Enums) => + { + "struct" + } + ErrorKind::Constructor => "struct or enum variant", + ErrorKind::Datatype + if !context + .env + .supports_feature(context.current_package, FeatureGate::Enums) => + { + "struct" + } + ErrorKind::Datatype => "struct or enum", + ErrorKind::Function => "function", + ErrorKind::Constant if single_name => "local or constant", + ErrorKind::Constant => "constant", + ErrorKind::ModuleMember => "module member", + ErrorKind::Variable => "variable", + ErrorKind::Term if single_name => "variable or constant", + ErrorKind::Term => "local, constant, or enum variant (of no arguments)", + ErrorKind::Module => "module", + ErrorKind::ApplyNamed => "struct or enum variant", + ErrorKind::PatternTerm if single_name => "variable or constant", + ErrorKind::PatternTerm => "loca, constant, or enum variant (of no arguments)", + } + } + + fn unbound_error_code(&self, single_name: bool) -> codes::NameResolution { + use codes::NameResolution as NR; + match self { + ErrorKind::Type => NR::UnboundType, + ErrorKind::Constructor if single_name => NR::UnboundUnscopedName, + ErrorKind::Constructor => NR::UnboundModuleMember, + ErrorKind::Datatype if single_name => NR::UnboundUnscopedName, + ErrorKind::Datatype => NR::UnboundModuleMember, + ErrorKind::Function if single_name => NR::UnboundUnscopedName, + ErrorKind::Function => NR::UnboundModuleMember, + ErrorKind::Constant if single_name => NR::InvalidPosition, + ErrorKind::Constant => NR::UnboundModuleMember, + ErrorKind::Term if single_name => NR::UnboundVariable, + ErrorKind::Term => NR::UnboundModuleMember, + ErrorKind::ModuleMember => NR::UnboundModuleMember, + ErrorKind::Variable => NR::UnboundVariable, + ErrorKind::Module => NR::UnboundModule, + ErrorKind::ApplyNamed => NR::UnboundModuleMember, + ErrorKind::PatternTerm if single_name => NR::UnboundVariable, + ErrorKind::PatternTerm => NR::UnboundModuleMember, + } + } + + fn invalid_form_error_code(&self, _single_name: bool) -> codes::NameResolution { + use codes::NameResolution as NR; + match self { + ErrorKind::PatternTerm => NR::InvalidPattern, + ErrorKind::Type + | ErrorKind::Constructor + | ErrorKind::Constant + | ErrorKind::Function + | ErrorKind::Term + | ErrorKind::Variable + | ErrorKind::Module + | ErrorKind::ModuleMember + | ErrorKind::ApplyNamed + | ErrorKind::Datatype => NR::InvalidPosition, + } + } +} + +fn make_unbound_name_error_msg( + context: &Context, + expected: &ErrorKind, + is_single_name: bool, + name: impl std::fmt::Display, +) -> String { + format!( + "Unbound {} '{name}'", + expected.kind_name(context, is_single_name) + ) +} + +fn make_unbound_module_error( + context: &Context, + loc: Loc, + mident: impl std::fmt::Display, +) -> Diagnostic { + let msg = make_unbound_name_error_msg(context, &ErrorKind::Module, true, mident); + diag!( + ErrorKind::Module.unbound_error_code(/* is_single_name */ true), + (loc, msg) + ) +} + +fn make_unbound_local_name_error( + context: &Context, + expected: &ErrorKind, + loc: Loc, + name: impl std::fmt::Display, +) -> Diagnostic { + let base_msg = make_unbound_name_error_msg(context, expected, true, name); + let msg = format!("{base_msg} in current scope"); + diag!( + expected.unbound_error_code(/* is_single_name */ true), + (loc, msg) + ) +} + +fn make_unbound_module_member_error( + context: &Context, + expected: &Option, + loc: Loc, + mident: ModuleIdent, + name: impl std::fmt::Display, +) -> Diagnostic { + let expected = expected.as_ref().unwrap_or(&ErrorKind::ModuleMember); + let same_module = context.current_module == Some(mident); + let (prefix, postfix) = if same_module { + ("", " in current scope".to_string()) + } else { + ("Invalid module access. ", format!(" in module '{mident}'")) + }; + let msg = format!( + "{prefix}{}{postfix}", + make_unbound_name_error_msg(context, expected, same_module, name) + ); + diag!( + expected.unbound_error_code(/* is_single_name */ false), + (loc, msg) + ) +} + +fn make_invalid_module_member_kind_error( + context: &Context, + expected: &ErrorKind, + loc: Loc, + actual: &ResolvedModuleMember, +) -> Diagnostic { + let mident = actual.mident(); + let same_module = context.current_module == Some(mident); + let (prefix, postfix) = if same_module { + ("", " in current scope".to_string()) + } else { + ("Invalid module access. ", format!(" in module '{mident}'")) + }; + let msg = format!( + "{prefix}Expected a {}, but found {} '{}'{postfix}", + expected.kind_name(context, same_module), + actual, + actual.name_symbol() + ); + diag!( + expected.invalid_form_error_code(/* is_single_name */ false), + (loc, msg) + ) +} + +#[allow(dead_code)] fn arity_string(arity: usize) -> &'static str { match arity { 0 => "", @@ -1366,34 +1738,46 @@ fn explicit_use_fun( ty, method, } = e; - let m_f_opt = match resolve_function(context, ResolveFunctionCase::UseFun, loc, function, None) - { - ResolvedFunction::Module(mf) => { + let m_f_opt = match context.resolve_use_fun_function(function) { + ResolvedUseFunFunction::Module(mf) => { let ResolvedModuleFunction { - module, - function, - ty_args, + mident, + name, + tyarg_arity: _, + arity: _, } = *mf; - assert!(ty_args.is_none()); - Some((module, function)) + Some((mident, name)) } - ResolvedFunction::Builtin(_) => { + ResolvedUseFunFunction::Builtin(_) => { let msg = "Invalid 'use fun'. Cannot use a builtin function as a method"; context .env .add_diag(diag!(Declarations::InvalidUseFun, (loc, msg))); None } - ResolvedFunction::Var(_) => { - unreachable!("ICE this case should be excluded from ResolveFunctionCase::UseFun") - } - ResolvedFunction::Unbound => { + ResolvedUseFunFunction::Unbound => { assert!(context.env.has_errors()); None } }; let ty_loc = ty.loc; + + // check use fun scope first to avoid some borrow pain nastiness let tn_opt = match context.resolve_type(ty) { + rt @ (ResolvedType::ModuleType(_) | ResolvedType::BuiltinType(_)) + if check_use_fun_scope(context, &loc, &is_public, &rt) => + { + rt + } + ResolvedType::ModuleType(_) | ResolvedType::BuiltinType(_) => { + assert!(context.env.has_errors()); + ResolvedType::Unbound + } + ty @ (ResolvedType::TParam(_, _) | ResolvedType::Hole | ResolvedType::Unbound) => ty, + }; + let tn_opt = match tn_opt { + ResolvedType::BuiltinType(bt_) => Some(N::TypeName_::Builtin(sp(ty.loc, bt_))), + ResolvedType::ModuleType(mt) => Some(N::TypeName_::ModuleType(mt.mident(), mt.name())), ResolvedType::Unbound => { assert!(context.env.has_errors()); None @@ -1421,37 +1805,9 @@ fn explicit_use_fun( )); None } - ResolvedType::BuiltinType(bt_) => Some(N::TypeName_::Builtin(sp(ty.loc, bt_))), - ResolvedType::ModuleType(mt) => match mt.module_type { - ModuleType::Struct(stype) => Some(N::TypeName_::ModuleType( - stype.original_mident, - DatatypeName(mt.original_type_name), - )), - ModuleType::Enum(etype) => Some(N::TypeName_::ModuleType( - etype.original_mident, - DatatypeName(mt.original_type_name), - )), - }, }; let tn_ = tn_opt?; let tn = sp(ty.loc, tn_); - if let Some(pub_loc) = is_public { - let current_module = context.current_module; - if let Err(def_loc_opt) = use_fun_module_defines(context, current_module, &tn) { - let msg = "Invalid 'use fun'. Cannot publicly associate a function with a \ - type defined in another module"; - let pub_msg = format!( - "Declared '{}' here. Consider removing to make a local 'use fun' instead", - Visibility::PUBLIC - ); - let mut diag = diag!(Declarations::InvalidUseFun, (loc, msg), (pub_loc, pub_msg)); - if let Some(def_loc) = def_loc_opt { - diag.add_secondary_label((def_loc, "Type defined in another module here")); - } - context.env.add_diag(diag); - return None; - } - } let target_function = m_f_opt?; let use_fun = N::UseFun { loc, @@ -1465,13 +1821,57 @@ fn explicit_use_fun( Some((tn, method, use_fun)) } +fn check_use_fun_scope( + context: &mut Context, + use_fun_loc: &Loc, + is_public: &Option, + rtype: &ResolvedType, +) -> bool { + let Some(pub_loc) = is_public else { + return true; + }; + let current_module = context.current_module; + let Err(def_loc_opt) = use_fun_module_defines(context, use_fun_loc, current_module, rtype) + else { + return true; + }; + + let msg = "Invalid 'use fun'. Cannot publicly associate a function with a \ + type defined in another module"; + let pub_msg = format!( + "Declared '{}' here. Consider removing to make a local 'use fun' instead", + Visibility::PUBLIC + ); + let mut diag = diag!( + Declarations::InvalidUseFun, + (*use_fun_loc, msg), + (*pub_loc, pub_msg) + ); + if let Some(def_loc) = def_loc_opt { + diag.add_secondary_label((def_loc, "Type defined in another module here")); + } + context.env.add_diag(diag); + false +} + fn use_fun_module_defines( context: &mut Context, + use_fun_loc: &Loc, specified: Option, - tn: &N::TypeName, + rtype: &ResolvedType, ) -> Result<(), Option> { - match &tn.value { - N::TypeName_::Builtin(sp!(_, b_)) => { + match rtype { + ResolvedType::ModuleType(mtype) => { + if specified + .as_ref() + .is_some_and(|mident| mident == &mtype.mident()) + { + Ok(()) + } else { + Err(Some(mtype.decl_loc())) + } + } + ResolvedType::BuiltinType(b_) => { let definer_opt = context.env.primitive_definer(*b_); match (definer_opt, &specified) { (None, _) => Err(None), @@ -1485,26 +1885,11 @@ fn use_fun_module_defines( } } } - N::TypeName_::ModuleType(m, n) => { - if specified.as_ref().is_some_and(|n| n == m) { - Ok(()) - } else { - let mod_type = context - .scoped_types - .get(m) - .unwrap() - .get(&n.value()) - .unwrap(); - Err(Some(mod_type.decl_loc())) - } - } - ty @ N::TypeName_::Multiple(_) => { - let msg = format!( - "ICE tuple type {} should not be reachable from use fun", - debug_display!(ty) - ); - context.env.add_diag(ice!((tn.loc, msg))); - // This is already reporting a bug, so let's continue for lack of something better to do. + ResolvedType::TParam(_, _) | ResolvedType::Hole | ResolvedType::Unbound => { + context.env.add_diag(ice!(( + *use_fun_loc, + "Tried to validate use fun for invalid type" + ))); Ok(()) } } @@ -1553,7 +1938,7 @@ fn friend(context: &mut Context, mident: ModuleIdent, friend: E::Friend) -> Opti (mident.loc, "Cannot declare the module itself as a friend"), )); None - } else if context.resolve_module(&mident) { + } else if context.valid_module(&mident) { Some(friend) } else { assert!(context.env.has_errors()); @@ -1922,18 +2307,12 @@ fn type_parameter( tp } -fn opt_types_with_arity_check String>( +fn types_opt( context: &mut Context, case: TypeAnnotation, - loc: Loc, - name_f: F, - ty_args: Option>, - arity: usize, + tys: Option>, ) -> Option> { - ty_args.map(|etys| { - let tys = types(context, case, etys); - check_type_argument_arity(context, loc, name_f, tys, arity) - }) + tys.map(|tys| types(context, case, tys)) } fn types(context: &mut Context, case: TypeAnnotation, tys: Vec) -> Vec { @@ -1955,93 +2334,85 @@ fn type_(context: &mut Context, case: TypeAnnotation, sp!(loc, ety_): E::Type) - assert!(context.env.has_errors()); NT::UnresolvedError } - ET::Apply(ma, tys) => match context.resolve_type(ma) { - RT::Unbound => { - assert!(context.env.has_errors()); - NT::UnresolvedError - } - RT::Hole => { - let case_str_opt = match case { - TypeAnnotation::StructField => { - Some(("Struct fields", " or consider adding a new type parameter")) - } - TypeAnnotation::VariantField => Some(( - "Enum variant fields", - " or consider adding a new type parameter", - )), - TypeAnnotation::ConstantSignature => Some(("Constants", "")), - TypeAnnotation::FunctionSignature => { - Some(("Functions", " or consider adding a new type parameter")) - } - TypeAnnotation::MacroSignature | TypeAnnotation::Expression => None, - }; - if let Some((case_str, help_str)) = case_str_opt { - let msg = format!( - "Invalid usage of a placeholder for type inference '_'. \ - {case_str} require fully specified types. Replace '_' with a specific type{help_str}" - ); - let mut diag = diag!(NameResolution::InvalidTypeAnnotation, (loc, msg)); - if let TypeAnnotation::FunctionSignature = case { - diag.add_note("Only 'macro' functions can use '_' in their signatures"); - } - context.env.add_diag(diag); + ET::Apply(ma, tys) => { + let original_loc = ma.loc; + match context.resolve_type(ma) { + RT::Unbound => { + assert!(context.env.has_errors()); NT::UnresolvedError - } else { - // replaced with a type variable during type instantiation - NT::Anything } - } - RT::BuiltinType(bn_) => { - let name_f = || format!("{}", &bn_); - let arity = bn_.tparam_constraints(loc).len(); - let tys = types(context, case, tys); - let tys = check_type_argument_arity(context, loc, name_f, tys, arity); - NT::builtin_(sp(ma.loc, bn_), tys) - } - RT::TParam(_, tp) => { - if !tys.is_empty() { - context.env.add_diag(diag!( - NameResolution::NamePositionMismatch, - (loc, "Generic type parameters cannot take type arguments"), - )); - NT::UnresolvedError - } else { - if context.translating_fun { - context.used_fun_tparams.insert(tp.id); - } - NT::Param(tp) - } - } - RT::ModuleType(mt) => { - let ResolvedModuleType { - original_loc, - original_type_name, - module_type, - } = *mt; - let (tn, arity) = match module_type { - ModuleType::Struct(stype) => { - let tn = sp( - original_loc, - NN::ModuleType(stype.original_mident, DatatypeName(original_type_name)), - ); - let arity = stype.arity; - (tn, arity) + RT::Hole => { + let case_str_opt = match case { + TypeAnnotation::StructField => { + Some(("Struct fields", " or consider adding a new type parameter")) + } + TypeAnnotation::VariantField => Some(( + "Enum variant fields", + " or consider adding a new type parameter", + )), + TypeAnnotation::ConstantSignature => Some(("Constants", "")), + TypeAnnotation::FunctionSignature => { + Some(("Functions", " or consider adding a new type parameter")) + } + TypeAnnotation::MacroSignature | TypeAnnotation::Expression => None, + }; + if let Some((case_str, help_str)) = case_str_opt { + let msg = format!( + "Invalid usage of a placeholder for type inference '_'. \ + {case_str} require fully specified types. Replace '_' with a specific type{help_str}" + ); + let mut diag = diag!(NameResolution::InvalidTypeAnnotation, (loc, msg)); + if let TypeAnnotation::FunctionSignature = case { + diag.add_note("Only 'macro' functions can use '_' in their signatures"); + } + context.env.add_diag(diag); + NT::UnresolvedError + } else { + // replaced with a type variable during type instantiation + NT::Anything } - ModuleType::Enum(etype) => { - let tn = sp( - original_loc, - NN::ModuleType(etype.original_mident, DatatypeName(original_type_name)), - ); - let arity = etype.arity; - (tn, arity) + } + RT::BuiltinType(bn_) => { + let name_f = || format!("{}", &bn_); + let arity = bn_.tparam_constraints(loc).len(); + let tys = types(context, case, tys); + let tys = check_type_instantiation_arity(context, loc, name_f, tys, arity); + NT::builtin_(sp(ma.loc, bn_), tys) + } + RT::TParam(_, tp) => { + if !tys.is_empty() { + context.env.add_diag(diag!( + NameResolution::NamePositionMismatch, + (loc, "Generic type parameters cannot take type arguments"), + )); + NT::UnresolvedError + } else { + if context.translating_fun { + context.used_fun_tparams.insert(tp.id); + } + NT::Param(tp) } - }; - let tys = types(context, case, tys); - let name_f = || format!("{}", tn); - let tys = check_type_argument_arity(context, loc, name_f, tys, arity); - NT::Apply(None, tn, tys) + } + RT::ModuleType(mt) => { + let (tn, arity) = match mt { + ResolvedDatatype::Struct(stype) => { + let tn = sp(original_loc, NN::ModuleType(stype.mident, stype.name)); + let arity = stype.tyarg_arity; + (tn, arity) + } + ResolvedDatatype::Enum(etype) => { + let tn = sp(original_loc, NN::ModuleType(etype.mident, etype.name)); + let arity = etype.tyarg_arity; + (tn, arity) + } + }; + let tys = types(context, case, tys); + let name_f = || format!("{}", tn); + let tys = check_type_instantiation_arity(context, loc, name_f, tys, arity); + NT::Apply(None, tn, tys) + } } - }, + } ET::Fun(tys, ty) => { let tys = types(context, case, tys); let ty = Box::new(type_(context, case, *ty)); @@ -2051,7 +2422,21 @@ fn type_(context: &mut Context, case: TypeAnnotation, sp!(loc, ety_): E::Type) - sp(loc, ty_) } -fn check_type_argument_arity String>( +fn types_opt_with_instantiation_arity_check String>( + context: &mut Context, + case: TypeAnnotation, + loc: Loc, + name_f: F, + ty_args: Option>, + arity: usize, +) -> Option> { + ty_args.map(|etys| { + let tys = types(context, case, etys); + check_type_instantiation_arity(context, loc, name_f, tys, arity) + }) +} + +fn check_type_instantiation_arity String>( context: &mut Context, loc: Loc, name_f: F, @@ -2147,48 +2532,62 @@ fn exp(context: &mut Context, e: Box) -> Box { let ne_ = match e_ { EE::Unit { trailing } => NE::Unit { trailing }, EE::Value(val) => NE::Value(val), - EE::Name(sp!(_, E::ModuleAccess_::Name(v)), None) if !is_constant_name(&v.value) => { - match context.resolve_local( - eloc, - NameResolution::UnboundVariable, - |name| format!("Unbound variable '{name}'"), - v, - ) { - None => { - debug_assert!(context.env.has_errors()); - NE::UnresolvedError + EE::Name(ma, tyargs_opt) => { + match context.resolve_term(ma) { + ResolvedTerm::Constant(const_) => { + exp_types_opt_with_arity_check( + context, + ma.loc, + || "Constants cannot take type arguments".to_string(), + eloc, + tyargs_opt, + 0, + ); + N::Exp_::Constant(const_.mident, const_.name) } - Some(nv) => NE::Var(nv), - } - } - EE::Name(ma @ sp!(_, E::ModuleAccess_::Variant(_, _)), etys_opt) => { - context - .env - .check_feature(context.current_package, FeatureGate::Enums, eloc); - let Some((m, n, ty)) = context.resolve_datatype_constructor(ma, "construction") else { - assert!(context.env.has_errors()); - return Box::new(sp(eloc, NE::UnresolvedError)); - }; - let tys_opt = opt_types_with_arity_check( - context, - TypeAnnotation::Expression, - eloc, - || format!("{}::{}", &m, &n), - etys_opt, - ty.type_arity(), - ); - check_constructor_form(context, eloc, ConstructorForm::None, "instantiation", &ty); - match ty { - ResolvedConstructor::Struct(_, _stype) => { + ResolvedTerm::Variant(vtype) => { + let tys_opt = types_opt_with_instantiation_arity_check( + context, + TypeAnnotation::Expression, + eloc, + || format!("{}::{}", &vtype.mident, &vtype.enum_name), + tyargs_opt, + vtype.tyarg_arity, + ); + check_constructor_form( + context, + eloc, + ConstructorForm::None, + "instantiation", + &ResolvedConstructor::Variant(vtype.clone()), + ); + NE::PackVariant( + vtype.mident, + vtype.enum_name, + vtype.name, + tys_opt, + UniqueMap::new(), + ) + } + ResolvedTerm::Var(var) => { + exp_types_opt_with_arity_check( + context, + ma.loc, + || "Variables cannot take type arguments".to_string(), + eloc, + tyargs_opt, + 0, + ); + NE::Var(*var) + } + ResolvedTerm::Unbound => { + // Just for the errors + types_opt(context, TypeAnnotation::Expression, tyargs_opt); assert!(context.env.has_errors()); NE::UnresolvedError } - ResolvedConstructor::Variant(_etype, variant, _, _) => { - NE::PackVariant(m, n, variant, tys_opt, UniqueMap::new()) - } } } - EE::Name(ma, None) => access_constant(context, ma), EE::IfElse(eb, et, ef) => NE::IfElse(exp(context, eb), exp(context, et), exp(context, ef)), EE::Match(esubject, sp!(_aloc, arms)) if arms.is_empty() => { @@ -2363,23 +2762,31 @@ fn exp(context: &mut Context, e: Box) -> Box { EE::Pack(ma, etys_opt, efields) => { // Process fields for errors either way. let fields = efields.map(|_, (idx, e)| (idx, *exp(context, Box::new(e)))); - let Some((m, n, ty)) = context.resolve_datatype_constructor(ma, "construction") else { + let Some(ctor) = context.resolve_datatype_constructor(ma, "construction") else { assert!(context.env.has_errors()); return Box::new(sp(eloc, NE::UnresolvedError)); }; - let tys_opt = opt_types_with_arity_check( + let tys_opt = types_opt_with_instantiation_arity_check( context, TypeAnnotation::Expression, eloc, - || format!("{}::{}", &m, &n), + || ctor.type_name(), etys_opt, - ty.type_arity(), + ctor.type_arity(), + ); + check_constructor_form( + context, + eloc, + ConstructorForm::Braces, + "instantiation", + &ctor, ); - check_constructor_form(context, eloc, ConstructorForm::Braces, "instantiation", &ty); - match ty { - ResolvedConstructor::Struct(_, _stype) => NE::Pack(m, n, tys_opt, fields), - ResolvedConstructor::Variant(_etype, variant, _vloc, _vfields) => { - NE::PackVariant(m, n, variant, tys_opt, fields) + match ctor { + ResolvedConstructor::Struct(stype) => { + NE::Pack(stype.mident, stype.name, tys_opt, fields) + } + ResolvedConstructor::Variant(vtype) => { + NE::PackVariant(vtype.mident, vtype.enum_name, vtype.name, tys_opt, fields) } } } @@ -2397,128 +2804,16 @@ fn exp(context: &mut Context, e: Box) -> Box { }, EE::Cast(e, t) => NE::Cast( - exp(context, e), - type_(context, TypeAnnotation::Expression, t), - ), - EE::Annotate(e, t) => NE::Annotate( - exp(context, e), - type_(context, TypeAnnotation::Expression, t), - ), - - EE::Call(ma, is_macro, etys_opt, rhs) if context.resolves_to_datatype(&ma) => { - context - .env - .check_feature(context.current_package, FeatureGate::PositionalFields, eloc); - report_invalid_macro(context, is_macro, "Datatypes"); - let nes = call_args(context, rhs); - let Some((m, n, ty)) = context.resolve_datatype_constructor(ma, "construction") else { - assert!(context.env.has_errors()); - return Box::new(sp(eloc, NE::UnresolvedError)); - }; - let tys_opt = etys_opt.map(|etys| { - let tys = types(context, TypeAnnotation::Expression, etys); - let name_f = || format!("{}::{}", &m, &n); - check_type_argument_arity(context, eloc, name_f, tys, ty.type_arity()) - }); - check_constructor_form(context, eloc, ConstructorForm::Parens, "instantiation", &ty); - let fields = - UniqueMap::maybe_from_iter(nes.value.into_iter().enumerate().map(|(idx, e)| { - let field = Field::add_loc(e.loc, format!("{idx}").into()); - (field, (idx, e)) - })) - .unwrap(); - match ty { - ResolvedConstructor::Struct(_, _stype) => NE::Pack(m, n, tys_opt, fields), - ResolvedConstructor::Variant(_etype, variant, _vloc, _vfields) => { - NE::PackVariant(m, n, variant, tys_opt, fields) - } - } - } - EE::Call(ma, is_macro, tys_opt, rhs) => { - use N::BuiltinFunction_ as BF; - let ty_args = tys_opt.map(|tys| types(context, TypeAnnotation::Expression, tys)); - let mut nes = call_args(context, rhs); - match resolve_function(context, ResolveFunctionCase::Call, eloc, ma, ty_args) { - ResolvedFunction::Builtin(sp!(bloc, BF::Assert(_))) => { - if is_macro.is_none() { - let dep_msg = format!( - "'{}' function syntax has been deprecated and will be removed", - BF::ASSERT_MACRO - ); - // TODO make this a tip/hint? - let help_msg = format!( - "Replace with '{0}!'. '{0}' has been replaced with a '{0}!' built-in \ - macro so that arguments are no longer eagerly evaluated", - BF::ASSERT_MACRO - ); - context.env.add_diag(diag!( - Uncategorized::DeprecatedWillBeRemoved, - (bloc, dep_msg), - (bloc, help_msg), - )); - } - // If no abort code is given for the assert, we add in the abort code as the - // bitset-line-number if `CleverAssertions` is set. - if nes.value.len() == 1 && is_macro.is_some() { - context.env.check_feature( - context.current_package, - FeatureGate::CleverAssertions, - bloc, - ); - nes.value.push(sp( - bloc, - NE::ErrorConstant { - line_number_loc: bloc, - }, - )); - } - NE::Builtin(sp(bloc, BF::Assert(is_macro)), nes) - } - ResolvedFunction::Builtin(bf @ sp!(_, BF::Freeze(_))) => { - if let Some(mloc) = is_macro { - let msg = format!( - "Unexpected macro invocation. '{}' cannot be invoked as a \ - macro", - bf.value.display_name() - ); - context - .env - .add_diag(diag!(TypeSafety::InvalidCallTarget, (mloc, msg))); - } - NE::Builtin(bf, nes) - } - - ResolvedFunction::Module(mf) => { - if let Some(mloc) = is_macro { - context.env.check_feature( - context.current_package, - FeatureGate::MacroFuns, - mloc, - ); - } - let ResolvedModuleFunction { - module, - function, - ty_args, - } = *mf; - NE::ModuleCall(module, function, is_macro, ty_args, nes) - } - ResolvedFunction::Var(v) => { - if let Some(mloc) = is_macro { - let msg = - "Unexpected macro invocation. Bound lambdas cannot be invoked as \ - a macro"; - context - .env - .add_diag(diag!(TypeSafety::InvalidCallTarget, (mloc, msg))); - } - NE::VarCall(v, nes) - } - ResolvedFunction::Unbound => { - assert!(context.env.has_errors()); - NE::UnresolvedError - } - } + exp(context, e), + type_(context, TypeAnnotation::Expression, t), + ), + EE::Annotate(e, t) => NE::Annotate( + exp(context, e), + type_(context, TypeAnnotation::Expression, t), + ), + + EE::Call(ma, is_macro, tys_opt, rhs) => { + resolve_call(context, eloc, ma, is_macro, tys_opt, rhs) } EE::MethodCall(edot, n, is_macro, tys_opt, rhs) => match dotted(context, *edot) { None => { @@ -2539,15 +2834,14 @@ fn exp(context: &mut Context, e: Box) -> Box { } }, EE::Vector(vec_loc, tys_opt, rhs) => { - let ty_args = tys_opt.map(|tys| types(context, TypeAnnotation::Expression, tys)); let nes = call_args(context, rhs); - let ty_opt = check_builtin_ty_args_impl( + let ty_opt = exp_types_opt_with_arity_check( context, vec_loc, || "Invalid 'vector' instantation".to_string(), eloc, + tys_opt, 1, - ty_args, ) .map(|mut v| { assert!(v.len() == 1); @@ -2561,7 +2855,7 @@ fn exp(context: &mut Context, e: Box) -> Box { NE::UnresolvedError } // `Name` matches name variants only allowed in specs (we handle the allowed ones above) - e @ (EE::Index(..) | EE::Quant(..) | EE::Name(_, Some(_))) => { + e @ (EE::Index(..) | EE::Quant(..)) => { let mut diag = ice!(( eloc, "ICE compiler should not have parsed this form as a specification" @@ -2574,16 +2868,6 @@ fn exp(context: &mut Context, e: Box) -> Box { Box::new(sp(eloc, ne_)) } -fn access_constant(context: &mut Context, ma: E::ModuleAccess) -> N::Exp_ { - match context.resolve_constant(ma) { - None => { - assert!(context.env.has_errors()); - N::Exp_::UnresolvedError - } - Some((m, c)) => N::Exp_::Constant(m, c), - } -} - fn dotted(context: &mut Context, edot: E::ExpDotted) -> Option { let sp!(loc, edot_) = edot; let nedot_ = match edot_ { @@ -2673,9 +2957,9 @@ fn check_constructor_form( }; } - let name = ty.name(); + let name = ty.name_symbol(); match ty { - RC::Struct(_, stype) => match form { + RC::Struct(stype) => match form { CF::None => { let (form_upcase, form) = if stype.field_info.is_positional() { (POSNL_UPCASE, POSNL) @@ -2716,82 +3000,74 @@ fn check_constructor_form( } CF::Braces => (), }, - RC::Variant(_, _, vloc, vfields) => match form { - CF::None if vfields.is_empty() => (), - CF::None => { - let (form_upcase, form) = if vfields.is_positional() { - (POSNL_UPCASE, POSNL) - } else { - (NAMED_UPCASE, NAMED) - }; - let msg = invalid_inst_msg!("variant", form_upcase, form); - let mut diag = diag!( - NameResolution::PositionalCallMismatch, - (loc, &msg), - (*vloc, defn_loc_error(&name)), - ); - if vfields.is_positional() { - diag.add_note(posnl_note!()); - } else { + RC::Variant(variant) => { + let vloc = variant.decl_loc; + let vfields = &variant.field_info; + match form { + CF::None if vfields.is_empty() => (), + CF::None => { + let (form_upcase, form) = if vfields.is_positional() { + (POSNL_UPCASE, POSNL) + } else { + (NAMED_UPCASE, NAMED) + }; + let msg = invalid_inst_msg!("variant", form_upcase, form); + let mut diag = diag!( + NameResolution::PositionalCallMismatch, + (loc, &msg), + (vloc, defn_loc_error(&name)), + ); + if vfields.is_positional() { + diag.add_note(posnl_note!()); + } else { + diag.add_note(named_note!()); + } + context.env.add_diag(diag); + } + CF::Parens if vfields.is_empty() => { + let msg = invalid_inst_msg!("variant", EMPTY_UPCASE, EMPTY); + let mut diag = diag!( + NameResolution::PositionalCallMismatch, + (loc, msg), + (vloc, defn_loc_error(&name)), + ); + diag.add_note(format!("Remove '()' arguments from this {position}")); + context.env.add_diag(diag); + } + CF::Parens if vfields.is_positional() => (), + CF::Parens => { + let msg = invalid_inst_msg!("variant", NAMED_UPCASE, NAMED); + let mut diag = diag!( + NameResolution::PositionalCallMismatch, + (loc, &msg), + (vloc, defn_loc_error(&name)), + ); diag.add_note(named_note!()); + context.env.add_diag(diag); } - context.env.add_diag(diag); - } - CF::Parens if vfields.is_empty() => { - let msg = invalid_inst_msg!("variant", EMPTY_UPCASE, EMPTY); - let mut diag = diag!( - NameResolution::PositionalCallMismatch, - (loc, msg), - (*vloc, defn_loc_error(&name)), - ); - diag.add_note(format!("Remove '()' arguments from this {position}")); - context.env.add_diag(diag); - } - CF::Parens if vfields.is_positional() => (), - CF::Parens => { - let msg = invalid_inst_msg!("variant", NAMED_UPCASE, NAMED); - let mut diag = diag!( - NameResolution::PositionalCallMismatch, - (loc, &msg), - (*vloc, defn_loc_error(&name)), - ); - diag.add_note(named_note!()); - context.env.add_diag(diag); - } - CF::Braces if vfields.is_empty() => { - let msg = invalid_inst_msg!("variant", EMPTY_UPCASE, EMPTY); - let mut diag = diag!( - NameResolution::PositionalCallMismatch, - (loc, msg), - (*vloc, defn_loc_error(&name)), - ); - diag.add_note(format!("Remove '{{ }}' arguments from this {position}")); - context.env.add_diag(diag); - } - CF::Braces if vfields.is_positional() => { - let msg = invalid_inst_msg!("variant", POSNL_UPCASE, POSNL); - let mut diag = diag!( - NameResolution::PositionalCallMismatch, - (loc, &msg), - (*vloc, defn_loc_error(&name)), - ); - diag.add_note(posnl_note!()); - context.env.add_diag(diag); + CF::Braces if vfields.is_empty() => { + let msg = invalid_inst_msg!("variant", EMPTY_UPCASE, EMPTY); + let mut diag = diag!( + NameResolution::PositionalCallMismatch, + (loc, msg), + (vloc, defn_loc_error(&name)), + ); + diag.add_note(format!("Remove '{{ }}' arguments from this {position}")); + context.env.add_diag(diag); + } + CF::Braces if vfields.is_positional() => { + let msg = invalid_inst_msg!("variant", POSNL_UPCASE, POSNL); + let mut diag = diag!( + NameResolution::PositionalCallMismatch, + (loc, &msg), + (vloc, defn_loc_error(&name)), + ); + diag.add_note(posnl_note!()); + context.env.add_diag(diag); + } + CF::Braces => (), } - CF::Braces => (), - }, - } -} - -fn report_invalid_macro(context: &mut Context, is_macro: Option, kind: &str) { - if let Some(mloc) = is_macro { - let msg = format!( - "Unexpected macro invocation. {} cannot be invoked as macros", - kind - ); - context - .env - .add_diag(diag!(NameResolution::PositionalCallMismatch, (mloc, msg))); + } } } @@ -3149,23 +3425,23 @@ fn match_pattern(context: &mut Context, in_pat: Box) -> Box { - let Some((m, n, ty)) = context.resolve_datatype_constructor(name, "pattern") else { + let Some(ctor) = context.resolve_datatype_constructor(name, "pattern") else { assert!(context.env.has_errors()); return Box::new(sp(ploc, NP::ErrorPat)); }; - let tys_opt = opt_types_with_arity_check( + let tys_opt = types_opt_with_instantiation_arity_check( context, TypeAnnotation::Expression, ploc, - || format!("{}::{}", &m, &n), + || ctor.type_name(), etys_opt, - ty.type_arity(), + ctor.type_arity(), ); - check_constructor_form(context, ploc, ConstructorForm::Parens, "pattern", &ty); + check_constructor_form(context, ploc, ConstructorForm::Parens, "pattern", &ctor); - let field_info = ty.field_info(); + let field_info = ctor.field_info(); let n_pats = args .value .into_iter() @@ -3180,35 +3456,34 @@ fn match_pattern(context: &mut Context, in_pat: Box) -> Box { - NP::Struct(m, n, tys_opt, result_args) + match ctor { + ResolvedConstructor::Struct(stype) => { + NP::Struct(stype.mident, stype.name, tys_opt, args) } - ResolvedConstructor::Variant(_etype, variant, _vloc, _vfields) => { - NP::Variant(m, n, variant, tys_opt, result_args) + ResolvedConstructor::Variant(vtype) => { + NP::Variant(vtype.mident, vtype.enum_name, vtype.name, tys_opt, args) } } } EP::NamedConstructor(name, etys_opt, args, ellipsis) => { - let Some((m, n, ty)) = context.resolve_datatype_constructor(name, "pattern") else { + let Some(ctor) = context.resolve_datatype_constructor(name, "pattern") else { assert!(context.env.has_errors()); return Box::new(sp(ploc, NP::ErrorPat)); }; - let tys_opt = opt_types_with_arity_check( + let tys_opt = types_opt_with_instantiation_arity_check( context, TypeAnnotation::Expression, ploc, - || format!("{}::{}", &m, &n), + || ctor.type_name(), etys_opt, - ty.type_arity(), + ctor.type_arity(), ); - check_constructor_form(context, ploc, ConstructorForm::Braces, "pattern", &ty); + check_constructor_form(context, ploc, ConstructorForm::Braces, "pattern", &ctor); - let field_info = ty.field_info(); + let field_info = ctor.field_info(); let mut args = args.map(|_, (idx, p)| (idx, *match_pattern(context, Box::new(p)))); // If we have an ellipsis fill in any missing patterns if let Some(ellipsis_loc) = ellipsis { @@ -3217,56 +3492,56 @@ fn match_pattern(context: &mut Context, in_pat: Box) -> Box NP::Struct(m, n, tys_opt, args), - ResolvedConstructor::Variant(_etype, variant, _vloc, _vfields) => { - NP::Variant(m, n, variant, tys_opt, args) + match ctor { + ResolvedConstructor::Struct(stype) => { + NP::Struct(stype.mident, stype.name, tys_opt, args) + } + ResolvedConstructor::Variant(vtype) => { + NP::Variant(vtype.mident, vtype.enum_name, vtype.name, tys_opt, args) } } } EP::ModuleAccessName(name, etys_opt) => { - let resolves_to_constant = context.resolves_to_constant(&name); - let resolves_to_datatype = context.resolves_to_datatype(&name); - if resolves_to_constant && resolves_to_datatype { - // If this happened, we should have already thrown an error about it in - // expansion alias map construction. - assert!(context.env.has_errors()); - return Box::new(sp(ploc, NP::ErrorPat)); - } else if resolves_to_constant { - let Some((m, n)) = context.resolve_constant(name) else { - unreachable!() - }; - if etys_opt.is_some() { - context.env.add_diag(diag!( - NameResolution::TooManyTypeArguments, - (ploc, "Constants in patterns do not take type arguments") - )); - } - NP::Constant(m, n) - } else { - let Some((m, n, ty)) = context.resolve_datatype_constructor(name, "pattern") else { - assert!(context.env.has_errors()); - return Box::new(sp(ploc, NP::ErrorPat)); - }; - let tys_opt = opt_types_with_arity_check( - context, - TypeAnnotation::Expression, - ploc, - || format!("{}::{}", &m, &n), - etys_opt, - ty.type_arity(), - ); - - match ty { - ResolvedConstructor::Struct(_name, _stype) => { - // No need to chck is_empty / is_positional because typing will report the errors. - NP::Struct(m, n, tys_opt, UniqueMap::new()) + match context.resolve_pattern_term(name) { + ResolvedPatternTerm::Constant(const_) => { + if etys_opt.is_some() { + context.env.add_diag(diag!( + NameResolution::TooManyTypeArguments, + (ploc, "Constants in patterns do not take type arguments") + )); } - ResolvedConstructor::Variant(_etype, variant, _vloc, _vfields) => { - // No need to chck is_empty / is_positional because typing will report the errors. - NP::Variant(m, n, variant, tys_opt, UniqueMap::new()) + NP::Constant(const_.mident, const_.name) + } + ResolvedPatternTerm::Constructor(ctor) => { + let tys_opt = types_opt_with_instantiation_arity_check( + context, + TypeAnnotation::Expression, + ploc, + || ctor.type_name(), + etys_opt, + ctor.type_arity(), + ); + match *ctor { + ResolvedConstructor::Struct(stype) => { + // No need to chck is_empty / is_positional because typing will report the errors. + NP::Struct(stype.mident, stype.name, tys_opt, UniqueMap::new()) + } + ResolvedConstructor::Variant(vtype) => { + // No need to chck is_empty / is_positional because typing will report the errors. + NP::Variant( + vtype.mident, + vtype.enum_name, + vtype.name, + tys_opt, + UniqueMap::new(), + ) + } } } + ResolvedPatternTerm::Unbound => { + assert!(context.env.has_errors()); + NP::ErrorPat + } } } EP::Binder(_, binder) if binder.is_underscore() => NP::Wildcard, @@ -3384,23 +3659,47 @@ fn lvalue( C::Bind => "deconstructing binding", C::Assign => "deconstructing assignment", }; - let (m, sn, tys_opt, field_info) = - context.resolve_struct_name(loc, msg, tn, etys_opt)?; - if field_info.is_positional() && !matches!(efields, E::FieldBindings::Positional(_)) { - let msg = "Invalid deconstruction. Positional struct field declarations require \ - positional deconstruction"; - context - .env - .add_diag(diag!(NameResolution::PositionalCallMismatch, (loc, msg))); - } - - if !field_info.is_positional() && matches!(efields, E::FieldBindings::Positional(_)) { - let msg = "Invalid deconstruction. Named struct field declarations require \ - named deconstruction"; - context - .env - .add_diag(diag!(NameResolution::PositionalCallMismatch, (loc, msg))); - } + let stype = match context.resolve_datatype_constructor(tn, "left-hand side") { + Some(ctor @ ResolvedConstructor::Struct(_)) => { + check_constructor_form( + context, + loc, + match efields { + E::FieldBindings::Named(_, _) => ConstructorForm::Braces, + E::FieldBindings::Positional(_) => ConstructorForm::Parens, + }, + "deconstruction", + &ctor, + ); + let ResolvedConstructor::Struct(stype) = ctor else { + unreachable!() + }; + stype + } + Some(ResolvedConstructor::Variant(variant)) => { + context.env.add_diag(diag!( + NameResolution::NamePositionMismatch, + (tn.loc, format!("Invalid {}. Expected a struct", msg)), + ( + variant.enum_name.loc(), + format!("But '{}' is an enum", variant.enum_name) + ) + )); + return None; + } + None => { + assert!(context.env.has_errors()); + return None; + } + }; + let tys_opt = types_opt_with_instantiation_arity_check( + context, + TypeAnnotation::Expression, + loc, + || format!("{}::{}", &stype.mident, &stype.name), + etys_opt, + stype.tyarg_arity, + ); let make_ignore = |loc| { let var = sp(loc, Symbol::from("_")); let name = E::ModuleAccess::new(loc, E::ModuleAccess_::Name(var)); @@ -3410,7 +3709,7 @@ fn lvalue( E::FieldBindings::Named(mut efields, ellipsis) => { if let Some(ellipsis_loc) = ellipsis { expand_named_ellipsis( - &field_info, + &stype.field_info, loc, ellipsis_loc, &mut efields, @@ -3421,20 +3720,21 @@ fn lvalue( efields } E::FieldBindings::Positional(lvals) => { - let fields = field_info.field_count(); + let fields = stype.field_info.field_count(); let missing = (fields as isize) - lvals.len() as isize; let expanded_lvals = expand_positional_ellipsis(missing, lvals, make_ignore); UniqueMap::maybe_from_iter(expanded_lvals.into_iter()).unwrap() } }; + let nfields = UniqueMap::maybe_from_opt_iter(efields.into_iter().map(|(k, (idx, inner))| { Some((k, (idx, lvalue(context, seen_locals, case, inner)?))) }))?; NL::Unpack( - m, - sn, + stype.mident, + stype.name, tys_opt, nfields.expect("ICE fields were already unique"), ) @@ -3500,161 +3800,238 @@ fn lvalue_list( )) } -fn resolve_function( +//************************************************************************************************** +// Resolvers +//************************************************************************************************** + +fn resolve_call( context: &mut Context, - case: ResolveFunctionCase, - loc: Loc, - sp!(mloc, ma_): E::ModuleAccess, - ty_args: Option>, -) -> ResolvedFunction { - use E::ModuleAccess_ as EA; - match (ma_, case) { - (EA::ModuleAccess(m, n), _) => match context.resolve_module_function(mloc, &m, &n) { - None => { - assert!(context.env.has_errors()); - ResolvedFunction::Unbound - } - Some(_) => ResolvedFunction::Module(Box::new(ResolvedModuleFunction { - module: m, - function: FunctionName(n), - ty_args, - })), - }, - (EA::Name(n), _) if N::BuiltinFunction_::all_names().contains(&n.value) => { - match resolve_builtin_function(context, loc, &n, ty_args) { - None => { - assert!(context.env.has_errors()); - ResolvedFunction::Unbound - } - Some(f) => ResolvedFunction::Builtin(sp(mloc, f)), + call_loc: Loc, + fun_name: E::ModuleAccess, + is_macro: Option, + in_tyargs_opt: Option>, + in_args: Spanned>, +) -> N::Exp_ { + use N::BuiltinFunction_ as B; + + let subject_loc = fun_name.loc; + let mut args = call_args(context, in_args); + + match context.resolve_call_subject(fun_name) { + ResolvedCallSubject::Function(mf) => { + let ResolvedModuleFunction { + mident, + name, + tyarg_arity: _, + arity: _, + } = *mf; + // TODO This is a weird place to check this feature gate. + if let Some(mloc) = is_macro { + context + .env + .check_feature(context.current_package, FeatureGate::MacroFuns, mloc); } + // TODO. We could check arities here, but we don't; type dones that, instead. + let tyargs_opt = types_opt(context, TypeAnnotation::Expression, in_tyargs_opt); + N::Exp_::ModuleCall(mident, name, is_macro, tyargs_opt, args) } - (EA::Name(n), ResolveFunctionCase::UseFun) => { - context.env.add_diag(diag!( - NameResolution::UnboundUnscopedName, - (n.loc, format!("Unbound function '{}' in current scope", n)), - )); - ResolvedFunction::Unbound - } - (EA::Name(n), ResolveFunctionCase::Call) => { - match context.resolve_local( - n.loc, - NameResolution::UnboundUnscopedName, - |n| format!("Unbound function '{}' in current scope", n), - n, - ) { - None => { - assert!(context.env.has_errors()); - ResolvedFunction::Unbound + ResolvedCallSubject::Builtin(bf) => { + let builtin_ = match &bf.fun.value { + B::Freeze(_) => { + check_is_not_macro(context, is_macro, B::FREEZE); + let tyargs_opt = exp_types_opt_with_arity_check( + context, + subject_loc, + || format!("Invalid call to builtin function: '{}'", B::FREEZE), + call_loc, + in_tyargs_opt, + 1, + ); + match tyargs_opt.as_deref() { + Some([ty]) => B::Freeze(Some(ty.clone())), + Some(_tys) => { + context + .env + .add_diag(ice!((call_loc, "Builtin tyarg arity failure"))); + return N::Exp_::UnresolvedError; + } + None => B::Freeze(None), + } } - Some(v) => { - if ty_args.is_some() { - context.env.add_diag(diag!( - NameResolution::TooManyTypeArguments, - (mloc, "Invalid lambda call. Expected zero type arguments"), + B::Assert(_) => { + if is_macro.is_none() { + let dep_msg = format!( + "'{}' function syntax has been deprecated and will be removed", + B::ASSERT_MACRO + ); + // TODO make this a tip/hint? + let help_msg = format!( + "Replace with '{0}!'. '{0}' has been replaced with a '{0}!' built-in \ + macro so that arguments are no longer eagerly evaluated", + B::ASSERT_MACRO + ); + let mut diag = + diag!(Uncategorized::DeprecatedWillBeRemoved, (call_loc, dep_msg),); + diag.add_note(help_msg); + context.env.add_diag(diag); + } + exp_types_opt_with_arity_check( + context, + subject_loc, + || format!("Invalid call to builtin function: '{}'", B::ASSERT_MACRO), + call_loc, + in_tyargs_opt, + 0, + ); + // If no abort code is given for the assert, we add in the abort code as the + // bitset-line-number if `CleverAssertions` is set. + if args.value.len() == 1 && is_macro.is_some() { + context.env.check_feature( + context.current_package, + FeatureGate::CleverAssertions, + subject_loc, + ); + args.value.push(sp( + call_loc, + N::Exp_::ErrorConstant { + line_number_loc: subject_loc, + }, )); } - ResolvedFunction::Var(v) + B::Assert(is_macro) + } + }; + N::Exp_::Builtin(sp(subject_loc, builtin_), args) + } + ResolvedCallSubject::Constructor(_) => { + context.env.check_feature( + context.current_package, + FeatureGate::PositionalFields, + call_loc, + ); + report_invalid_macro(context, is_macro, "Datatypes"); + let Some(ctor) = context.resolve_datatype_constructor(fun_name, "construction") else { + assert!(context.env.has_errors()); + return N::Exp_::UnresolvedError; + }; + let tyargs_opt = exp_types_opt_with_arity_check( + context, + subject_loc, + || "Invalid call to constructor".to_string(), + call_loc, + in_tyargs_opt, + ctor.type_arity(), + ); + check_constructor_form( + context, + call_loc, + ConstructorForm::Parens, + "instantiation", + &ctor, + ); + let fields = + UniqueMap::maybe_from_iter(args.value.into_iter().enumerate().map(|(idx, e)| { + let field = Field::add_loc(e.loc, format!("{idx}").into()); + (field, (idx, e)) + })) + .unwrap(); + match ctor { + ResolvedConstructor::Struct(stype) => { + N::Exp_::Pack(stype.mident, stype.name, tyargs_opt, fields) } + ResolvedConstructor::Variant(vtype) => N::Exp_::PackVariant( + vtype.mident, + vtype.enum_name, + vtype.name, + tyargs_opt, + fields, + ), } } - (EA::Variant(_, _), _) => { - context.env.add_diag(ice!(( - mloc, - "Tried to resovle variant '{}' as a function in current scope" - ),)); - ResolvedFunction::Unbound + ResolvedCallSubject::Var(var) => { + check_is_not_macro(context, is_macro, &var.value.name); + let tyargs_opt = types_opt(context, TypeAnnotation::Expression, in_tyargs_opt); + if tyargs_opt.is_some() { + context.env.add_diag(diag!( + NameResolution::TooManyTypeArguments, + ( + subject_loc, + "Invalid lambda call. Expected zero type arguments" + ), + )); + } + N::Exp_::VarCall(sp(subject_loc, var.value), args) } + ResolvedCallSubject::Unbound => N::Exp_::UnresolvedError, } } -fn resolve_builtin_function( - context: &mut Context, - loc: Loc, - b: &Name, - ty_args: Option>, -) -> Option { - use N::{BuiltinFunction_ as B, BuiltinFunction_::*}; - Some(match b.value.as_str() { - B::FREEZE => Freeze(check_builtin_ty_arg(context, loc, b, ty_args)), - B::ASSERT_MACRO => { - check_builtin_ty_args(context, loc, b, 0, ty_args); - Assert(/* is_macro, set by caller */ None) - } - _ => { - context.env.add_diag(diag!( - NameResolution::UnboundUnscopedName, - (b.loc, format!("Unbound function: '{}'", b)), - )); - return None; - } - }) -} +//************************************************************************************************** +// General helpers +//************************************************************************************************** -fn check_builtin_ty_arg( - context: &mut Context, - loc: Loc, - b: &Name, - ty_args: Option>, -) -> Option { - let res = check_builtin_ty_args(context, loc, b, 1, ty_args); - res.map(|mut v| { - assert!(v.len() == 1); - v.pop().unwrap() - }) +fn check_is_not_macro(context: &mut Context, is_macro: Option, name: &str) { + if let Some(mloc) = is_macro { + let msg = format!( + "Unexpected macro invocation. '{}' cannot be invoked as a \ + macro", + name + ); + context + .env + .add_diag(diag!(TypeSafety::InvalidCallTarget, (mloc, msg))); + } } -fn check_builtin_ty_args( - context: &mut Context, - loc: Loc, - b: &Name, - arity: usize, - ty_args: Option>, -) -> Option> { - check_builtin_ty_args_impl( - context, - b.loc, - || format!("Invalid call to builtin function: '{}'", b), - loc, - arity, - ty_args, - ) +fn report_invalid_macro(context: &mut Context, is_macro: Option, kind: &str) { + if let Some(mloc) = is_macro { + let msg = format!( + "Unexpected macro invocation. {} cannot be invoked as macros", + kind + ); + context + .env + .add_diag(diag!(NameResolution::PositionalCallMismatch, (mloc, msg))); + } } -fn check_builtin_ty_args_impl( +fn exp_types_opt_with_arity_check( context: &mut Context, msg_loc: Loc, fmsg: impl Fn() -> String, - targs_loc: Loc, + tyarg_error_loc: Loc, + tyargs_opt: Option>, arity: usize, - ty_args: Option>, ) -> Option> { - let mut msg_opt = None; - ty_args.map(|mut args| { - let args_len = args.len(); - if args_len != arity { - let diag_code = if args_len > arity { - NameResolution::TooManyTypeArguments - } else { - NameResolution::TooFewTypeArguments - }; - let msg = msg_opt.get_or_insert_with(fmsg); - let targs_msg = format!("Expected {} type argument(s) but got {}", arity, args_len); - context - .env - .add_diag(diag!(diag_code, (msg_loc, msg), (targs_loc, targs_msg))); - } + let tyargs_opt = tyargs_opt.map(|etys| types(context, TypeAnnotation::Expression, etys)); + let Some(mut args) = tyargs_opt else { + return None; + }; + let args_len = args.len(); + if args_len != arity { + let diag_code = if args_len > arity { + NameResolution::TooManyTypeArguments + } else { + NameResolution::TooFewTypeArguments + }; + let msg = fmsg(); + let targs_msg = format!("Expected {} type argument(s) but got {}", arity, args_len); + context.env.add_diag(diag!( + diag_code, + (msg_loc, msg), + (tyarg_error_loc, targs_msg) + )); + } - while args.len() > arity { - args.pop(); - } + while args.len() > arity { + args.pop(); + } - while args.len() < arity { - args.push(sp(targs_loc, N::Type_::UnresolvedError)); - } + while args.len() < arity { + args.push(sp(tyarg_error_loc, N::Type_::UnresolvedError)); + } - args - }) + Some(args) } //************************************************************************************************** diff --git a/external-crates/move/crates/move-compiler/src/shared/mod.rs b/external-crates/move/crates/move-compiler/src/shared/mod.rs index 994d7da4ae705..ffefbab30b01d 100644 --- a/external-crates/move/crates/move-compiler/src/shared/mod.rs +++ b/external-crates/move/crates/move-compiler/src/shared/mod.rs @@ -98,6 +98,10 @@ pub trait TName: Eq + Ord + Clone { fn drop_loc(self) -> (Self::Loc, Self::Key); fn add_loc(loc: Self::Loc, key: Self::Key) -> Self; fn borrow(&self) -> (&Self::Loc, &Self::Key); + fn with_loc(self, loc: Self::Loc) -> Self { + let (_old_loc, base) = self.drop_loc(); + Self::add_loc(loc, base) + } } pub trait Identifier { diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/constants_dont_shadow_leading_access_struct.exp b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/constants_dont_shadow_leading_access_struct.exp index 96999c17cb732..5227ec5d0a431 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/constants_dont_shadow_leading_access_struct.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/constants_dont_shadow_leading_access_struct.exp @@ -1,9 +1,9 @@ error[E03006]: unexpected name in this position ┌─ tests/move_2024/expansion/constants_dont_shadow_leading_access_struct.move:15:13 │ + 3 │ public struct S() has copy, drop; + │ - But 'S' is an struct + · 15 │ S::foo(); // resolves to struct - │ ^ - │ │ - │ Invalid construction. Expected an enum - │ But 'S' is a struct + │ ^ Invalid construction. Expected an enum diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/use_nested_self_as_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/use_nested_self_as_invalid.exp index c5742e3e3aca5..f13dbad52c02d 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/use_nested_self_as_invalid.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/use_nested_self_as_invalid.exp @@ -33,9 +33,9 @@ error[E03006]: unexpected name in this position error[E03006]: unexpected name in this position ┌─ tests/move_2024/expansion/use_nested_self_as_invalid.move:11:9 │ + 9 │ struct X { f: X::S, f2: S } + │ - But 'X' is an struct +10 │ fun bar() { 11 │ X::foo(); - │ ^ - │ │ - │ Invalid construction. Expected an enum - │ But 'X' is a struct + │ ^ Invalid construction. Expected an enum diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/matching/invalid_head_ctor_3.exp b/external-crates/move/crates/move-compiler/tests/move_2024/matching/invalid_head_ctor_3.exp index d1c5d2341557e..815b5fbf264f8 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/matching/invalid_head_ctor_3.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/matching/invalid_head_ctor_3.exp @@ -1,6 +1,6 @@ -error[E03003]: unbound module member +error[E03022]: invalid usage position ┌─ tests/move_2024/matching/invalid_head_ctor_3.move:16:13 │ 16 │ l::a(_) => (), - │ ^^^^ Invalid module access. Expected a type, but found function 'a' in module '0x42::l' + │ ^^^^ Invalid module access. Expected a struct or enum, but found function 'a' in module '0x42::l' diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/clever_errors_line_assert.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/clever_errors_line_assert.exp index 0cf4cbfe7b5c5..c2c8602b077d9 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/naming/clever_errors_line_assert.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/clever_errors_line_assert.exp @@ -2,10 +2,9 @@ warning[W00001]: DEPRECATED. will be removed ┌─ tests/move_2024/naming/clever_errors_line_assert.move:3:9 │ 3 │ assert(false); - │ ^^^^^^ - │ │ - │ 'assert' function syntax has been deprecated and will be removed - │ Replace with 'assert!'. 'assert' has been replaced with a 'assert!' built-in macro so that arguments are no longer eagerly evaluated + │ ^^^^^^^^^^^^^ 'assert' function syntax has been deprecated and will be removed + │ + = Replace with 'assert!'. 'assert' has been replaced with a 'assert!' built-in macro so that arguments are no longer eagerly evaluated error[E04016]: too few arguments ┌─ tests/move_2024/naming/clever_errors_line_assert.move:3:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_struct_non_positional_unpack.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_struct_non_positional_unpack.exp index f37e787543cd1..b8b49cf7b19c7 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_struct_non_positional_unpack.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_struct_non_positional_unpack.exp @@ -9,8 +9,11 @@ warning[W09002]: unused variable error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/positional_struct_non_positional_unpack.move:7:13 │ +2 │ public struct Foo(u64) has copy, drop; + │ --- 'Foo' is declared here + · 7 │ let Foo { y: _ } = Foo(0); - │ ^^^^^^^^^^^^ Invalid deconstruction. Positional struct field declarations require positional deconstruction + │ ^^^^^^^^^^^^ Invalid struct deconstruction. Positional struct declarations require positional deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/naming/positional_struct_non_positional_unpack.move:7:13 @@ -35,8 +38,11 @@ warning[W09002]: unused variable error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/positional_struct_non_positional_unpack.move:14:9 │ + 2 │ public struct Foo(u64) has copy, drop; + │ --- 'Foo' is declared here + · 14 │ Foo { y: _ } = Foo(0); - │ ^^^^^^^^^^^^ Invalid deconstruction. Positional struct field declarations require positional deconstruction + │ ^^^^^^^^^^^^ Invalid struct deconstruction. Positional struct declarations require positional deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/naming/positional_struct_non_positional_unpack.move:14:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_unpack_of_non_positional_struct.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_unpack_of_non_positional_struct.exp index f26584c864852..df872c5e3b337 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_unpack_of_non_positional_struct.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/positional_unpack_of_non_positional_struct.exp @@ -1,8 +1,11 @@ error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/positional_unpack_of_non_positional_struct.move:7:13 │ +2 │ public struct Foo { field: u64 } has copy, drop; + │ --- 'Foo' is declared here + · 7 │ let Foo(_) = x; - │ ^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/naming/positional_unpack_of_non_positional_struct.move:7:13 @@ -19,8 +22,11 @@ error[E03010]: unbound field error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/positional_unpack_of_non_positional_struct.move:14:9 │ + 2 │ public struct Foo { field: u64 } has copy, drop; + │ --- 'Foo' is declared here + · 14 │ Foo(_) = x; - │ ^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/naming/positional_unpack_of_non_positional_struct.move:14:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/struct_ellipsis_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/struct_ellipsis_invalid.exp index 575525ec38f80..9699e3b2c5456 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/naming/struct_ellipsis_invalid.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/struct_ellipsis_invalid.exp @@ -1,14 +1,20 @@ error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:6:13 │ +3 │ public struct Y{} + │ - 'Y' is declared here + · 6 │ let Y(..) = x; - │ ^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:10:13 │ + 3 │ public struct Y{} + │ - 'Y' is declared here + · 10 │ let Y(x, ..) = x; - │ ^^^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E03010]: unbound field ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:10:13 @@ -27,20 +33,29 @@ warning[W09002]: unused variable error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:14:13 │ + 3 │ public struct Y{} + │ - 'Y' is declared here + · 14 │ let Y() = x; - │ ^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:18:13 │ + 2 │ public struct X() + │ - 'X' is declared here + · 18 │ let X{..} = x; - │ ^^^^^ Invalid deconstruction. Positional struct field declarations require positional deconstruction + │ ^^^^^ Invalid struct deconstruction. Positional struct declarations require positional deconstructions error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:22:13 │ + 2 │ public struct X() + │ - 'X' is declared here + · 22 │ let X{x, ..} = x; - │ ^^^^^^^^ Invalid deconstruction. Positional struct field declarations require positional deconstruction + │ ^^^^^^^^ Invalid struct deconstruction. Positional struct declarations require positional deconstructions error[E03010]: unbound field ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:22:13 @@ -59,6 +74,9 @@ warning[W09002]: unused variable error[E03013]: positional call mismatch ┌─ tests/move_2024/naming/struct_ellipsis_invalid.move:26:13 │ + 2 │ public struct X() + │ - 'X' is declared here + · 26 │ let X{} = x; - │ ^^^ Invalid deconstruction. Positional struct field declarations require positional deconstruction + │ ^^^ Invalid struct deconstruction. Positional struct declarations require positional deconstructions diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.exp index 44921b01793c8..3565b482bc84e 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.exp @@ -1,8 +1,11 @@ error[E03013]: positional call mismatch ┌─ tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.move:26:13 │ + 4 │ public struct Bar { + │ --- 'Bar' is declared here + · 26 │ let Bar(Foo(Bar(Foo(x, y)), z)) = y; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.move:26:13 @@ -19,8 +22,11 @@ error[E03010]: unbound field error[E03013]: positional call mismatch ┌─ tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.move:26:21 │ + 4 │ public struct Bar { + │ --- 'Bar' is declared here + · 26 │ let Bar(Foo(Bar(Foo(x, y)), z)) = y; - │ ^^^^^^^^^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^^^^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_2024/parser/invalid_positional_struct_unpack_deeply_nested.move:26:21 diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/long_path.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/long_path.exp index ba58d83fb8b20..0dbd2aa5175ab 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/long_path.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/long_path.exp @@ -25,9 +25,9 @@ error[E03006]: unexpected name in this position error[E03006]: unexpected name in this position ┌─ tests/move_2024/parser/long_path.move:11:9 │ + 2 │ public struct X { t: T } + │ - But 'X' is an struct + · 11 │ 0x42::m::X::X { t: abort 0 } - │ ^^^^^^^^^^ - │ │ │ - │ │ But 'X' is a struct - │ Invalid construction. Expected an enum + │ ^^^^^^^^^^ Invalid construction. Expected an enum diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_arg_assign.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_arg_assign.exp index 5eecf972698a9..83d5f26a3553d 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_arg_assign.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_arg_assign.exp @@ -2,7 +2,7 @@ error[E03022]: invalid usage position ┌─ tests/move_2024/parser/positional_struct_explicit_type_arg_assign.move:5:9 │ 5 │ Foo (_) = Foo(0); - │ ^^^ Expected local or constant, found struct 'Foo' in module '0x42::M' instead. + │ ^^^ Expected a local or constant, but found struct 'Foo' in current scope │ = Structs with positional arguments must be written as 'Foo( ... )' diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_args.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_args.exp index 8e11de563d07b..b399226af892b 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_args.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/positional_struct_explicit_type_args.exp @@ -2,7 +2,7 @@ error[E03022]: invalid usage position ┌─ tests/move_2024/parser/positional_struct_explicit_type_args.move:16:17 │ 16 │ let _ = Foo (0); - │ ^^^ Expected local or constant, found struct 'Foo' in module '0x42::M' instead. + │ ^^^ Expected a local or constant, but found struct 'Foo' in current scope │ = Structs with positional arguments must be written as 'Foo( ... )' diff --git a/external-crates/move/crates/move-compiler/tests/move_check/deprecated/assert_function.exp b/external-crates/move/crates/move-compiler/tests/move_check/deprecated/assert_function.exp index fa9a276e8a57d..459fc6571b681 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/deprecated/assert_function.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/deprecated/assert_function.exp @@ -2,8 +2,7 @@ warning[W00001]: DEPRECATED. will be removed ┌─ tests/move_check/deprecated/assert_function.move:3:9 │ 3 │ assert(true, 42); - │ ^^^^^^ - │ │ - │ 'assert' function syntax has been deprecated and will be removed - │ Replace with 'assert!'. 'assert' has been replaced with a 'assert!' built-in macro so that arguments are no longer eagerly evaluated + │ ^^^^^^^^^^^^^^^^ 'assert' function syntax has been deprecated and will be removed + │ + = Replace with 'assert!'. 'assert' has been replaced with a 'assert!' built-in macro so that arguments are no longer eagerly evaluated diff --git a/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.exp b/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.exp index df99543d4bccc..f198f83f1b3b5 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.exp @@ -7,11 +7,11 @@ error[E01002]: unexpected token │ Unexpected '{' │ Expected ';' -error[E03003]: unbound module member +error[E03022]: invalid usage position ┌─ tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.move:5:9 │ 5 │ foo() = 0; - │ ^^^ Invalid module access. Expected a type, but found function 'foo' in module '0x42::M' + │ ^^^ Expected a struct, but found function 'foo' in current scope error[E13001]: feature is not supported in specified edition ┌─ tests/move_check/expansion/invalid_unpack_assign_lhs_other_value.move:5:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.exp b/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.exp index ad4428bcdc53a..314137b0ee392 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.exp @@ -2,13 +2,13 @@ error[E03003]: unbound module member ┌─ tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.move:3:9 │ 3 │ Self::f {} = 0; - │ ^^^^^^^ Invalid module access. Unbound type 'f' in module '0x8675309::M' + │ ^^^^^^^ Unbound struct 'f' in current scope error[E03003]: unbound module member ┌─ tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.move:4:9 │ 4 │ Self::f() = 0; - │ ^^^^^^^ Invalid module access. Unbound type 'f' in module '0x8675309::M' + │ ^^^^^^^ Unbound struct 'f' in current scope error[E13001]: feature is not supported in specified edition ┌─ tests/move_check/expansion/invalid_unpack_assign_mdot_no_struct.move:4:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_check/expansion/pack_no_fields_single_block_other_expr.exp b/external-crates/move/crates/move-compiler/tests/move_check/expansion/pack_no_fields_single_block_other_expr.exp index f57171397604e..727a05056513d 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/expansion/pack_no_fields_single_block_other_expr.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/expansion/pack_no_fields_single_block_other_expr.exp @@ -2,7 +2,7 @@ error[E03022]: invalid usage position ┌─ tests/move_check/expansion/pack_no_fields_single_block_other_expr.move:6:18 │ 6 │ let _s = S 0; - │ ^ Expected local or constant, found struct 'S' in module '0x42::M' instead. + │ ^ Expected a local or constant, but found struct 'S' in current scope │ = Struct with named arguments must be written as 'S { ... }' @@ -19,7 +19,7 @@ error[E03022]: invalid usage position ┌─ tests/move_check/expansion/pack_no_fields_single_block_other_expr.move:7:18 │ 7 │ let _s = S f; - │ ^ Expected local or constant, found struct 'S' in module '0x42::M' instead. + │ ^ Expected a local or constant, but found struct 'S' in current scope │ = Struct with named arguments must be written as 'S { ... }' diff --git a/external-crates/move/crates/move-compiler/tests/move_check/expansion/unpack_assign_other_expr.exp b/external-crates/move/crates/move-compiler/tests/move_check/expansion/unpack_assign_other_expr.exp index 62ddc924f68e7..c7480d4407d60 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/expansion/unpack_assign_other_expr.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/expansion/unpack_assign_other_expr.exp @@ -9,8 +9,11 @@ error[E13001]: feature is not supported in specified edition error[E03013]: positional call mismatch ┌─ tests/move_check/expansion/unpack_assign_other_expr.move:6:9 │ +3 │ struct S { f: u64 } + │ - 'S' is declared here + · 6 │ S ( f ) = S { f: 0 }; - │ ^^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_check/expansion/unpack_assign_other_expr.move:6:9 @@ -36,7 +39,7 @@ error[E03022]: invalid usage position ┌─ tests/move_check/expansion/unpack_assign_other_expr.move:9:9 │ 9 │ S f = S { f: 0 }; - │ ^ Expected local or constant, found struct 'S' in module '0x42::M' instead. + │ ^ Expected a local or constant, but found struct 'S' in current scope │ = Struct with named arguments must be written as 'S { ... }' @@ -60,8 +63,11 @@ error[E13001]: feature is not supported in specified edition error[E03013]: positional call mismatch ┌─ tests/move_check/expansion/unpack_assign_other_expr.move:11:9 │ + 2 │ struct G {} + │ - 'G' is declared here + · 11 │ G () = G {}; - │ ^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E01002]: unexpected token ┌─ tests/move_check/expansion/unpack_assign_other_expr.move:12:12 diff --git a/external-crates/move/crates/move-compiler/tests/move_check/expansion/weird_apply_assign.exp b/external-crates/move/crates/move-compiler/tests/move_check/expansion/weird_apply_assign.exp index 4a3d7244abe17..b89f482db84bc 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/expansion/weird_apply_assign.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/expansion/weird_apply_assign.exp @@ -8,7 +8,7 @@ error[E03022]: invalid usage position ┌─ tests/move_check/expansion/weird_apply_assign.move:7:9 │ 7 │ S f = S { f: 0 }; - │ ^ Expected local or constant, found struct 'S' in module '0x42::M' instead. + │ ^ Expected a local or constant, but found struct 'S' in current scope │ = Struct with named arguments must be written as 'S { ... }' diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_constant.exp b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_constant.exp index 6a9b81772a70c..abf248976c9a5 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_constant.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_constant.exp @@ -8,7 +8,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_constant.move:5:17 │ 5 │ let y = Self::CONSTANT; y; - │ ^^^^^^^^^^^^^^ Invalid module access. Unbound constant 'CONSTANT' in module '0x42::M' + │ ^^^^^^^^^^^^^^ Unbound module member 'CONSTANT' in current scope error[E03005]: unbound unscoped name ┌─ tests/move_check/naming/unbound_constant.move:6:13 @@ -20,5 +20,5 @@ error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_constant.move:6:24 │ 6 │ 0 + CONSTANT + Self::CONSTANT; - │ ^^^^^^^^^^^^^^ Invalid module access. Unbound constant 'CONSTANT' in module '0x42::M' + │ ^^^^^^^^^^^^^^ Unbound module member 'CONSTANT' in current scope diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_module_name.exp b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_module_name.exp index bbbb37fcd7de6..51dd497c16889 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_module_name.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_module_name.exp @@ -2,23 +2,23 @@ error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_module_name.move:7:17 │ 7 │ let x = N::c; x; - │ ^^^^ Invalid module access. Unbound constant 'c' in module '0x42::N' + │ ^^^^ Invalid module access. Unbound module member 'c' in module '0x42::N' error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_module_name.move:8:17 │ 8 │ let y = Self::c; y; - │ ^^^^^^^ Invalid module access. Unbound constant 'c' in module '0x42::M' + │ ^^^^^^^ Unbound module member 'c' in current scope error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_module_name.move:9:13 │ 9 │ 0 + N::c + Self::c; - │ ^^^^ Invalid module access. Unbound constant 'c' in module '0x42::N' + │ ^^^^ Invalid module access. Unbound module member 'c' in module '0x42::N' error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_module_name.move:9:20 │ 9 │ 0 + N::c + Self::c; - │ ^^^^^^^ Invalid module access. Unbound constant 'c' in module '0x42::M' + │ ^^^^^^^ Unbound module member 'c' in current scope diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_current.exp b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_current.exp index 82bdd4e6b6c33..0fd116fbd5248 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_current.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_current.exp @@ -1,20 +1,20 @@ -error[E03003]: unbound module member +error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_current.move:2:16 │ 2 │ fun foo(s: Self::S): Self::S { - │ ^^^^^^^ Invalid module access. Unbound type 'S' in module '0x8675309::M' + │ ^^^^^^^ Unbound type 'S' in current scope -error[E03003]: unbound module member +error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_current.move:2:26 │ 2 │ fun foo(s: Self::S): Self::S { - │ ^^^^^^^ Invalid module access. Unbound type 'S' in module '0x8675309::M' + │ ^^^^^^^ Unbound type 'S' in current scope -error[E03003]: unbound module member +error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_current.move:7:16 │ 7 │ fun bar(): Self::S { - │ ^^^^^^^ Invalid module access. Unbound type 'S' in module '0x8675309::M' + │ ^^^^^^^ Unbound type 'S' in current scope error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_current.move:8:9 @@ -32,7 +32,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_struct_in_current.move:13:9 │ 13 │ Self::S {} = bar(); - │ ^^^^^^^ Invalid module access. Unbound type 'S' in module '0x8675309::M' + │ ^^^^^^^ Unbound struct 'S' in current scope error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_current.move:17:13 @@ -44,5 +44,5 @@ error[E03003]: unbound module member ┌─ tests/move_check/naming/unbound_struct_in_current.move:18:13 │ 18 │ let Self::S {} = bar(); - │ ^^^^^^^ Invalid module access. Unbound type 'S' in module '0x8675309::M' + │ ^^^^^^^ Unbound struct 'S' in current scope diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_module.exp b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_module.exp index cee40bf13885e..e7289a7c7319c 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_module.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/unbound_struct_in_module.exp @@ -1,10 +1,10 @@ -error[E03003]: unbound module member +error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_module.move:6:16 │ 6 │ fun foo(s: X::S): X::S { │ ^^^^ Invalid module access. Unbound type 'S' in module '0x2::X' -error[E03003]: unbound module member +error[E03004]: unbound type ┌─ tests/move_check/naming/unbound_struct_in_module.move:6:23 │ 6 │ fun foo(s: X::S): X::S { diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.exp index d4401fb43821c..db31af4e6dc59 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.exp @@ -2,7 +2,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:9:9 │ 9 │ X::S () = 0; - │ ^^^^ Invalid module access. Unbound type 'S' in module '0x2::X' + │ ^^^^ Invalid module access. Unbound struct 'S' in module '0x2::X' error[E13001]: feature is not supported in specified edition ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:9:9 @@ -16,7 +16,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:11:9 │ 11 │ X::S 0 = 0; - │ ^^^^ Invalid module access. Unbound constant 'S' in module '0x2::X' + │ ^^^^ Invalid module access. Unbound module member 'S' in module '0x2::X' error[E01002]: unexpected token ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:11:14 @@ -31,7 +31,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:13:9 │ 13 │ X::S { 0 } = 0; - │ ^^^^ Invalid module access. Unbound type 'S' in module '0x2::X' + │ ^^^^ Invalid module access. Unbound struct 'S' in module '0x2::X' error[E01002]: unexpected token ┌─ tests/move_check/parser/invalid_unpack_assign_rhs_not_fields.move:13:16 diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/positional_struct_unpack.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/positional_struct_unpack.exp index a105a3e148069..871cc84a2410f 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/positional_struct_unpack.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/positional_struct_unpack.exp @@ -9,8 +9,11 @@ error[E13001]: feature is not supported in specified edition error[E03013]: positional call mismatch ┌─ tests/move_check/parser/positional_struct_unpack.move:7:9 │ +4 │ struct Foo { f: u64 } + │ --- 'Foo' is declared here + · 7 │ Foo(_) = x; - │ ^^^^^^ Invalid deconstruction. Named struct field declarations require named deconstruction + │ ^^^^^^ Invalid struct deconstruction. Named struct declarations require named deconstructions error[E04016]: too few arguments ┌─ tests/move_check/parser/positional_struct_unpack.move:7:9 diff --git a/external-crates/move/crates/move-compiler/tests/move_check/typing/module_call_missing_function.exp b/external-crates/move/crates/move-compiler/tests/move_check/typing/module_call_missing_function.exp index d86facbffed7d..e67c0aacc55d7 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/typing/module_call_missing_function.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/typing/module_call_missing_function.exp @@ -2,7 +2,7 @@ error[E03003]: unbound module member ┌─ tests/move_check/typing/module_call_missing_function.move:13:9 │ 13 │ Self::fooo(); - │ ^^^^^^^^^^ Invalid module access. Unbound function 'fooo' in module '0x2::M' + │ ^^^^^^^^^^ Unbound function 'fooo' in current scope error[E03005]: unbound unscoped name ┌─ tests/move_check/typing/module_call_missing_function.move:14:9 From 9c5c7a0d040eba2ec514c451428c75d88e7982b4 Mon Sep 17 00:00:00 2001 From: Lu Zhang <8418040+longbowlu@users.noreply.github.com> Date: Wed, 29 May 2024 16:46:38 -0700 Subject: [PATCH 10/17] [bridge] Add print registration info in cli (#17908) ## Description Add a subcommand to display current registration info. ## Test plan ``` sui-bridge-cli print-sui-bridge-info --sui-rpc-url https://rpc.testnet.sui.io:443 ``` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- crates/sui-bridge-cli/src/lib.rs | 6 +++ crates/sui-bridge-cli/src/main.rs | 79 +++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/crates/sui-bridge-cli/src/lib.rs b/crates/sui-bridge-cli/src/lib.rs index 2d56f908e7e34..485ea13cdf7cf 100644 --- a/crates/sui-bridge-cli/src/lib.rs +++ b/crates/sui-bridge-cli/src/lib.rs @@ -90,6 +90,12 @@ pub enum BridgeCommand { #[clap(long = "eth-rpc-url")] eth_rpc_url: String, }, + /// Print current registration info + #[clap(name = "print-bridge-registration-info")] + PrintBridgeRegistrationInfo { + #[clap(long = "sui-rpc-url")] + sui_rpc_url: String, + }, /// Client to facilitate and execute Bridge actions #[clap(name = "client")] Client { diff --git a/crates/sui-bridge-cli/src/main.rs b/crates/sui-bridge-cli/src/main.rs index 1627b3db2eb09..abfcba21ec321 100644 --- a/crates/sui-bridge-cli/src/main.rs +++ b/crates/sui-bridge-cli/src/main.rs @@ -5,8 +5,11 @@ use clap::*; use ethers::providers::Middleware; use shared_crypto::intent::Intent; use shared_crypto::intent::IntentMessage; +use std::collections::HashMap; +use std::str::from_utf8; use std::sync::Arc; use sui_bridge::client::bridge_authority_aggregator::BridgeAuthorityAggregator; +use sui_bridge::crypto::BridgeAuthorityPublicKey; use sui_bridge::eth_transaction_builder::build_eth_transaction; use sui_bridge::sui_client::SuiClient; use sui_bridge::sui_transaction_builder::build_sui_transaction; @@ -21,8 +24,13 @@ use sui_bridge_cli::{ }; use sui_config::Config; use sui_sdk::SuiClient as SuiSdkClient; +use sui_sdk::SuiClientBuilder; use sui_types::bridge::BridgeChainId; +use sui_types::bridge::MoveTypeCommitteeMemberRegistration; +use sui_types::committee::TOTAL_VOTING_POWER; +use sui_types::crypto::AuthorityPublicKeyBytes; use sui_types::crypto::Signature; +use sui_types::crypto::ToFromBytes; use sui_types::transaction::Transaction; #[tokio::main] @@ -187,6 +195,77 @@ async fn main() -> anyhow::Result<()> { return Ok(()); } + BridgeCommand::PrintBridgeRegistrationInfo { sui_rpc_url } => { + let sui_bridge_client = SuiClient::::new(&sui_rpc_url).await?; + let bridge_summary = sui_bridge_client + .get_bridge_summary() + .await + .map_err(|e| anyhow::anyhow!("Failed to get bridge summary: {:?}", e))?; + let move_type_bridge_committee = bridge_summary.committee; + let sui_client = SuiClientBuilder::default().build(sui_rpc_url).await?; + let stakes = sui_client + .governance_api() + .get_committee_info(None) + .await? + .validators + .into_iter() + .collect::>(); + let names = sui_client + .governance_api() + .get_latest_sui_system_state() + .await? + .active_validators + .into_iter() + .map(|summary| { + let protocol_key = + AuthorityPublicKeyBytes::from_bytes(&summary.protocol_pubkey_bytes) + .unwrap(); + (summary.sui_address, (protocol_key, summary.name)) + }) + .collect::>(); + let mut authorities = vec![]; + for (_, member) in move_type_bridge_committee.member_registration { + let MoveTypeCommitteeMemberRegistration { + sui_address, + bridge_pubkey_bytes, + http_rest_url, + } = member; + let Ok(pubkey) = BridgeAuthorityPublicKey::from_bytes(&bridge_pubkey_bytes) else { + println!( + "Invalid bridge pubkey for validator {}: {:?}", + sui_address, bridge_pubkey_bytes + ); + continue; + }; + let Ok(base_url) = from_utf8(&http_rest_url) else { + println!( + "Invalid bridge http url for validator: {}: {:?}", + sui_address, http_rest_url + ); + continue; + }; + let base_url = base_url.to_string(); + + let (protocol_key, name) = names.get(&sui_address).unwrap(); + let stake = stakes.get(protocol_key).unwrap(); + authorities.push((name, sui_address, pubkey, base_url, stake)); + } + let total_stake = authorities + .iter() + .map(|(_, _, _, _, stake)| **stake) + .sum::(); + println!( + "Total registered stake: {}%", + total_stake as f32 / TOTAL_VOTING_POWER as f32 * 100.0 + ); + for (name, sui_address, pubkey, base_url, stake) in authorities { + println!( + "Name: {}, Sui Address: {}, Bridge Authority Pubkey: {}, Bridge Node URL: {}, Stake: {}", + name, sui_address, pubkey, base_url, stake + ); + } + } + BridgeCommand::Client { config_path, cmd } => { let config = BridgeCliConfig::load(config_path).expect("Couldn't load BridgeCliConfig"); let config = LoadedBridgeCliConfig::load(config).await?; From 8566b94610ded62d719ae212da3ca99c0ace8a16 Mon Sep 17 00:00:00 2001 From: Lu Zhang <8418040+longbowlu@users.noreply.github.com> Date: Wed, 29 May 2024 17:13:57 -0700 Subject: [PATCH 11/17] [bridge] add PrintBridgeCommitteeInfo subcommand and more governance command (#17965) ## Description 1. adds subcommand `PrintBridgeCommitteeInfo` 2. adds subcommands `AddTokensOnSui` and `AddTokensOnEvm` for governance ## Test plan tested all three locally. How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- crates/sui-bridge-cli/src/lib.rs | 95 ++++++++++++++++++++++++++++--- crates/sui-bridge-cli/src/main.rs | 88 +++++++++++++++++++++++++--- 2 files changed, 168 insertions(+), 15 deletions(-) diff --git a/crates/sui-bridge-cli/src/lib.rs b/crates/sui-bridge-cli/src/lib.rs index 485ea13cdf7cf..4258d12ffab54 100644 --- a/crates/sui-bridge-cli/src/lib.rs +++ b/crates/sui-bridge-cli/src/lib.rs @@ -16,14 +16,16 @@ use shared_crypto::intent::IntentMessage; use std::path::PathBuf; use std::str::FromStr; use std::sync::Arc; +use sui_bridge::abi::EthBridgeCommittee; use sui_bridge::abi::{eth_sui_bridge, EthSuiBridge}; use sui_bridge::crypto::BridgeAuthorityPublicKeyBytes; use sui_bridge::error::BridgeResult; use sui_bridge::sui_client::SuiBridgeClient; use sui_bridge::types::BridgeAction; use sui_bridge::types::{ - AssetPriceUpdateAction, BlocklistCommitteeAction, BlocklistType, EmergencyAction, - EmergencyActionType, EvmContractUpgradeAction, LimitUpdateAction, + AddTokensOnEvmAction, AddTokensOnSuiAction, AssetPriceUpdateAction, BlocklistCommitteeAction, + BlocklistType, EmergencyAction, EmergencyActionType, EvmContractUpgradeAction, + LimitUpdateAction, }; use sui_bridge::utils::{get_eth_signer_client, EthSigner}; use sui_config::Config; @@ -96,6 +98,12 @@ pub enum BridgeCommand { #[clap(long = "sui-rpc-url")] sui_rpc_url: String, }, + /// Print current committee info + #[clap(name = "print-bridge-committee-info")] + PrintBridgeCommitteeInfo { + #[clap(long = "sui-rpc-url")] + sui_rpc_url: String, + }, /// Client to facilitate and execute Bridge actions #[clap(name = "client")] Client { @@ -123,7 +131,7 @@ pub enum GovernanceClientCommands { nonce: u64, #[clap(name = "blocklist-type", long)] blocklist_type: BlocklistType, - #[clap(name = "pubkey-hex", long)] + #[clap(name = "pubkey-hex", use_value_delimiter = true, long)] pubkeys_hex: Vec, }, #[clap(name = "update-limit")] @@ -144,6 +152,30 @@ pub enum GovernanceClientCommands { #[clap(name = "new-usd-price", long)] new_usd_price: u64, }, + #[clap(name = "add-tokens-on-sui")] + AddTokensOnSui { + #[clap(name = "nonce", long)] + nonce: u64, + #[clap(name = "token-ids", use_value_delimiter = true, long)] + token_ids: Vec, + #[clap(name = "token-type-names", use_value_delimiter = true, long)] + token_type_names: Vec, + #[clap(name = "token-prices", use_value_delimiter = true, long)] + token_prices: Vec, + }, + #[clap(name = "add-tokens-on-evm")] + AddTokensOnEvm { + #[clap(name = "nonce", long)] + nonce: u64, + #[clap(name = "token-ids", use_value_delimiter = true, long)] + token_ids: Vec, + #[clap(name = "token-type-names", use_value_delimiter = true, long)] + token_addresses: Vec, + #[clap(name = "token-prices", use_value_delimiter = true, long)] + token_prices: Vec, + #[clap(name = "token-sui-decimals", use_value_delimiter = true, long)] + token_sui_decimals: Vec, + }, #[clap(name = "upgrade-evm-contract")] UpgradeEVMContract { #[clap(name = "nonce", long)] @@ -157,7 +189,7 @@ pub enum GovernanceClientCommands { #[clap(name = "function-selector", long)] function_selector: String, /// Params to be passed to the function, e.g. `420,false,hello` - #[clap(name = "params", long)] + #[clap(name = "params", use_value_delimiter = true, long)] params: Vec, }, } @@ -205,6 +237,43 @@ pub fn make_action(chain_id: BridgeChainId, cmd: &GovernanceClientCommands) -> B token_id: *token_id, new_usd_price: *new_usd_price, }), + GovernanceClientCommands::AddTokensOnSui { + nonce, + token_ids, + token_type_names, + token_prices, + } => { + assert_eq!(token_ids.len(), token_type_names.len()); + assert_eq!(token_ids.len(), token_prices.len()); + BridgeAction::AddTokensOnSuiAction(AddTokensOnSuiAction { + nonce: *nonce, + chain_id, + native: false, // only foreign tokens are supported now + token_ids: token_ids.clone(), + token_type_names: token_type_names.clone(), + token_prices: token_prices.clone(), + }) + } + GovernanceClientCommands::AddTokensOnEvm { + nonce, + token_ids, + token_addresses, + token_prices, + token_sui_decimals, + } => { + assert_eq!(token_ids.len(), token_addresses.len()); + assert_eq!(token_ids.len(), token_prices.len()); + assert_eq!(token_ids.len(), token_sui_decimals.len()); + BridgeAction::AddTokensOnEvmAction(AddTokensOnEvmAction { + nonce: *nonce, + native: true, // only eth native tokens are supported now + chain_id, + token_ids: token_ids.clone(), + token_addresses: token_addresses.clone(), + token_prices: token_prices.clone(), + token_sui_decimals: token_sui_decimals.clone(), + }) + } GovernanceClientCommands::UpgradeEVMContract { nonce, proxy_address, @@ -274,10 +343,10 @@ pub fn select_contract_address( config.eth_bridge_committee_proxy_address } GovernanceClientCommands::UpdateLimit { .. } => config.eth_bridge_limiter_proxy_address, - GovernanceClientCommands::UpdateAssetPrice { .. } => { - config.eth_bridge_limiter_proxy_address - } + GovernanceClientCommands::UpdateAssetPrice { .. } => config.eth_bridge_config_proxy_address, GovernanceClientCommands::UpgradeEVMContract { proxy_address, .. } => *proxy_address, + GovernanceClientCommands::AddTokensOnSui { .. } => unreachable!(), + GovernanceClientCommands::AddTokensOnEvm { .. } => config.eth_bridge_config_proxy_address, } } @@ -313,6 +382,8 @@ pub struct LoadedBridgeCliConfig { pub eth_bridge_proxy_address: EthAddress, /// Proxy address for BridgeCommittee deployed on Eth pub eth_bridge_committee_proxy_address: EthAddress, + /// Proxy address for BridgeConfig deployed on Eth + pub eth_bridge_config_proxy_address: EthAddress, /// Proxy address for BridgeLimiter deployed on Eth pub eth_bridge_limiter_proxy_address: EthAddress, /// Key pair for Sui operations @@ -365,6 +436,10 @@ impl LoadedBridgeCliConfig { let sui_bridge = EthSuiBridge::new(cli_config.eth_bridge_proxy_address, provider.clone()); let eth_bridge_committee_proxy_address: EthAddress = sui_bridge.committee().call().await?; let eth_bridge_limiter_proxy_address: EthAddress = sui_bridge.limiter().call().await?; + let eth_committee = + EthBridgeCommittee::new(eth_bridge_committee_proxy_address, provider.clone()); + let eth_bridge_committee_proxy_address: EthAddress = sui_bridge.committee().call().await?; + let eth_bridge_config_proxy_address: EthAddress = eth_committee.config().call().await?; let eth_address = eth_signer.address(); let eth_chain_id = provider.get_chainid().await?; @@ -379,6 +454,7 @@ impl LoadedBridgeCliConfig { eth_bridge_proxy_address: cli_config.eth_bridge_proxy_address, eth_bridge_committee_proxy_address, eth_bridge_limiter_proxy_address, + eth_bridge_config_proxy_address, sui_key, eth_signer, }) @@ -407,7 +483,10 @@ impl LoadedBridgeCliConfig { let gas = gases .into_iter() .find(|coin| coin.balance >= 5_000_000_000) - .ok_or(anyhow!("Did not find gas object with enough balance"))?; + .ok_or(anyhow!( + "Did not find gas object with enough balance for {}", + sui_client_address + ))?; println!("Using Gas object: {}", gas.coin_object_id); Ok((self.sui_key.copy(), sui_client_address, gas.object_ref())) } diff --git a/crates/sui-bridge-cli/src/main.rs b/crates/sui-bridge-cli/src/main.rs index abfcba21ec321..86c511cfdd0ed 100644 --- a/crates/sui-bridge-cli/src/main.rs +++ b/crates/sui-bridge-cli/src/main.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use std::str::from_utf8; use std::sync::Arc; use sui_bridge::client::bridge_authority_aggregator::BridgeAuthorityAggregator; -use sui_bridge::crypto::BridgeAuthorityPublicKey; +use sui_bridge::crypto::{BridgeAuthorityPublicKey, BridgeAuthorityPublicKeyBytes}; use sui_bridge::eth_transaction_builder::build_eth_transaction; use sui_bridge::sui_client::SuiClient; use sui_bridge::sui_transaction_builder::build_sui_transaction; @@ -26,7 +26,7 @@ use sui_config::Config; use sui_sdk::SuiClient as SuiSdkClient; use sui_sdk::SuiClientBuilder; use sui_types::bridge::BridgeChainId; -use sui_types::bridge::MoveTypeCommitteeMemberRegistration; +use sui_types::bridge::{MoveTypeCommitteeMember, MoveTypeCommitteeMemberRegistration}; use sui_types::committee::TOTAL_VOTING_POWER; use sui_types::crypto::AuthorityPublicKeyBytes; use sui_types::crypto::Signature; @@ -237,6 +237,7 @@ async fn main() -> anyhow::Result<()> { ); continue; }; + let eth_address = BridgeAuthorityPublicKeyBytes::from(&pubkey).to_eth_address(); let Ok(base_url) = from_utf8(&http_rest_url) else { println!( "Invalid bridge http url for validator: {}: {:?}", @@ -248,24 +249,97 @@ async fn main() -> anyhow::Result<()> { let (protocol_key, name) = names.get(&sui_address).unwrap(); let stake = stakes.get(protocol_key).unwrap(); - authorities.push((name, sui_address, pubkey, base_url, stake)); + authorities.push((name, sui_address, pubkey, eth_address, base_url, stake)); } let total_stake = authorities .iter() - .map(|(_, _, _, _, stake)| **stake) + .map(|(_, _, _, _, _, stake)| **stake) .sum::(); println!( "Total registered stake: {}%", total_stake as f32 / TOTAL_VOTING_POWER as f32 * 100.0 ); - for (name, sui_address, pubkey, base_url, stake) in authorities { + println!("Name, Sui Address, Eth Address, Pubkey, Base URL, Stake"); + for (name, sui_address, pubkey, eth_address, base_url, stake) in authorities { println!( - "Name: {}, Sui Address: {}, Bridge Authority Pubkey: {}, Bridge Node URL: {}, Stake: {}", - name, sui_address, pubkey, base_url, stake + "{}, {}, {}, {}, {}, {}", + name, sui_address, eth_address, pubkey, base_url, stake ); } } + BridgeCommand::PrintBridgeCommitteeInfo { sui_rpc_url } => { + let sui_bridge_client = SuiClient::::new(&sui_rpc_url).await?; + let bridge_summary = sui_bridge_client + .get_bridge_summary() + .await + .map_err(|e| anyhow::anyhow!("Failed to get bridge summary: {:?}", e))?; + let move_type_bridge_committee = bridge_summary.committee; + let sui_client = SuiClientBuilder::default().build(sui_rpc_url).await?; + let names = sui_client + .governance_api() + .get_latest_sui_system_state() + .await? + .active_validators + .into_iter() + .map(|summary| (summary.sui_address, summary.name)) + .collect::>(); + let mut authorities = vec![]; + for (_, member) in move_type_bridge_committee.members { + let MoveTypeCommitteeMember { + sui_address, + bridge_pubkey_bytes, + voting_power, + http_rest_url, + blocklisted, + } = member; + let Ok(pubkey) = BridgeAuthorityPublicKey::from_bytes(&bridge_pubkey_bytes) else { + println!( + "Invalid bridge pubkey for validator {}: {:?}", + sui_address, bridge_pubkey_bytes + ); + continue; + }; + let eth_address = BridgeAuthorityPublicKeyBytes::from(&pubkey).to_eth_address(); + let Ok(base_url) = from_utf8(&http_rest_url) else { + println!( + "Invalid bridge http url for validator: {}: {:?}", + sui_address, http_rest_url + ); + continue; + }; + let base_url = base_url.to_string(); + + let name = names.get(&sui_address).unwrap(); + authorities.push(( + name, + sui_address, + pubkey, + eth_address, + base_url, + voting_power, + blocklisted, + )); + } + let total_stake = authorities + .iter() + .map(|(_, _, _, _, _, stake, _)| *stake) + .sum::(); + println!( + "Total stake (static): {}%", + total_stake as f32 / TOTAL_VOTING_POWER as f32 * 100.0 + ); + + println!("Name, Sui Address, Eth Address, Pubkey, Base URL, Stake, Blocklisted"); + for (name, sui_address, pubkey, eth_address, base_url, stake, blocklisted) in + authorities + { + println!( + "{}, {}, 0x{:x}, {}, {}, {}, {}", + name, sui_address, eth_address, pubkey, base_url, stake, blocklisted + ); + } + } BridgeCommand::Client { config_path, cmd } => { let config = BridgeCliConfig::load(config_path).expect("Couldn't load BridgeCliConfig"); let config = LoadedBridgeCliConfig::load(config).await?; From 4f509fbaa9c2294f63ab0b0a193066ccda8cbdb5 Mon Sep 17 00:00:00 2001 From: Lu Zhang <8418040+longbowlu@users.noreply.github.com> Date: Wed, 29 May 2024 17:21:43 -0700 Subject: [PATCH 12/17] [bridge] add the first batch of metrics (#17966) ## Description as title. More to come ## Test plan tested by running bridge nodes locally --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- crates/sui-bridge/src/action_executor.rs | 26 +- crates/sui-bridge/src/e2e_tests/test_utils.rs | 7 +- crates/sui-bridge/src/lib.rs | 1 + crates/sui-bridge/src/main.rs | 3 +- crates/sui-bridge/src/metrics.rs | 84 +++ crates/sui-bridge/src/node.rs | 19 +- crates/sui-bridge/src/server/handler.rs | 6 - crates/sui-bridge/src/server/mock_handler.rs | 10 +- crates/sui-bridge/src/server/mod.rs | 591 +++++++++++------- 9 files changed, 503 insertions(+), 244 deletions(-) create mode 100644 crates/sui-bridge/src/metrics.rs diff --git a/crates/sui-bridge/src/action_executor.rs b/crates/sui-bridge/src/action_executor.rs index dd0af0ad3c315..0030cdc0cddd0 100644 --- a/crates/sui-bridge/src/action_executor.rs +++ b/crates/sui-bridge/src/action_executor.rs @@ -23,6 +23,7 @@ use crate::events::{ TokenTransferAlreadyApproved, TokenTransferAlreadyClaimed, TokenTransferApproved, TokenTransferClaimed, }; +use crate::metrics::BridgeMetrics; use crate::{ client::bridge_authority_aggregator::BridgeAuthorityAggregator, error::BridgeError, @@ -69,6 +70,7 @@ pub struct BridgeActionExecutor { gas_object_id: ObjectID, store: Arc, bridge_object_arg: ObjectArg, + metrics: Arc, } impl BridgeActionExecutorTrait for BridgeActionExecutor @@ -97,6 +99,7 @@ where key: SuiKeyPair, sui_address: SuiAddress, gas_object_id: ObjectID, + metrics: Arc, ) -> Self { let bridge_object_arg = sui_client .get_mutable_bridge_object_arg_must_succeed() @@ -109,6 +112,7 @@ where gas_object_id, sui_address, bridge_object_arg, + metrics, } } @@ -141,6 +145,7 @@ where let store_clone = self.store.clone(); let client_clone = self.sui_client.clone(); let mut tasks = vec![]; + let metrics = self.metrics.clone(); tasks.push(spawn_logged_monitored_task!( Self::run_signature_aggregation_loop( client_clone, @@ -163,6 +168,7 @@ where execution_tx_clone, execution_rx, self.bridge_object_arg, + metrics, ) )); (tasks, sender, execution_tx) @@ -294,6 +300,7 @@ where CertifiedBridgeActionExecutionWrapper, >, bridge_object_arg: ObjectArg, + metrics: Arc, ) { info!("Starting run_onchain_execution_loop"); // Get token id maps, this must succeed to continue. @@ -335,7 +342,7 @@ where ) { Ok(tx_data) => tx_data, Err(err) => { - // TODO: add mertrics + metrics.total_err_build_sui_transaction.inc(); error!( "Failed to build transaction for action {:?}: {:?}", certificate, err @@ -358,17 +365,22 @@ where .execute_transaction_block_with_effects(signed_tx) .await { - Ok(resp) => Self::handle_execution_effects(tx_digest, resp, &store, action).await, + Ok(resp) => { + Self::handle_execution_effects(tx_digest, resp, &store, action, &metrics).await + } // If the transaction did not go through, retry up to a certain times. Err(err) => { error!("Sui transaction failed at signing: {err:?}"); - + metrics.total_err_sui_transaction_submission.inc(); + let metrics_clone = metrics.clone(); // Do this in a separate task so we won't deadlock here let sender_clone = execution_queue_sender.clone(); spawn_logged_monitored_task!(async move { - // TODO: metrics + alerts // If it fails for too many times, log and ask for manual intervention. + metrics_clone + .total_err_sui_transaction_submission_too_many_failures + .inc(); if attempt_times >= MAX_EXECUTION_ATTEMPTS { error!("Manual intervention is required. Failed to collect execute transaction for bridge action after {MAX_EXECUTION_ATTEMPTS} attempts: {:?}", err); return; @@ -394,6 +406,7 @@ where response: SuiTransactionBlockResponse, store: &Arc, action: &BridgeAction, + metrics: &Arc, ) { let effects = response .effects @@ -429,7 +442,7 @@ where // the execution queue because retries are mostly likely going to fail anyway. // After human examination, the node should be restarted and fetch them from WAL. - // TODO metrics + alerts + metrics.total_err_sui_transaction_execution.inc(); error!(?tx_digest, "Manual intervention is needed. Sui transaction executed and failed with error: {error:?}"); } } @@ -1122,7 +1135,7 @@ mod tests { let committee = BridgeCommittee::new(authorities).unwrap(); let agg = Arc::new(BridgeAuthorityAggregator::new(Arc::new(committee))); - + let metrics = Arc::new(BridgeMetrics::new(®istry)); let executor = BridgeActionExecutor::new( sui_client.clone(), agg.clone(), @@ -1130,6 +1143,7 @@ mod tests { sui_key, sui_address, gas_object_ref.0, + metrics, ) .await; diff --git a/crates/sui-bridge/src/e2e_tests/test_utils.rs b/crates/sui-bridge/src/e2e_tests/test_utils.rs index a69fb5d60341f..9025831597fbf 100644 --- a/crates/sui-bridge/src/e2e_tests/test_utils.rs +++ b/crates/sui-bridge/src/e2e_tests/test_utils.rs @@ -10,6 +10,7 @@ use crate::utils::get_eth_signer_client; use crate::utils::EthSigner; use ethers::types::Address as EthAddress; use move_core_types::language_storage::StructTag; +use prometheus::Registry; use rand::rngs::SmallRng; use rand::{Rng, SeedableRng}; use serde::{Deserialize, Serialize}; @@ -718,7 +719,11 @@ pub(crate) async fn start_bridge_cluster( }; // Spawn bridge node in memory let config_clone = config.clone(); - handles.push(run_bridge_node(config_clone).await.unwrap()); + handles.push( + run_bridge_node(config_clone, Registry::new()) + .await + .unwrap(), + ); } handles } diff --git a/crates/sui-bridge/src/lib.rs b/crates/sui-bridge/src/lib.rs index 5eca9104afce7..bd52a06279e73 100644 --- a/crates/sui-bridge/src/lib.rs +++ b/crates/sui-bridge/src/lib.rs @@ -12,6 +12,7 @@ pub mod eth_client; pub mod eth_syncer; pub mod eth_transaction_builder; pub mod events; +pub mod metrics; pub mod node; pub mod orchestrator; pub mod server; diff --git a/crates/sui-bridge/src/main.rs b/crates/sui-bridge/src/main.rs index 70792b8e961e5..fdd9e5d558b3a 100644 --- a/crates/sui-bridge/src/main.rs +++ b/crates/sui-bridge/src/main.rs @@ -42,6 +42,5 @@ async fn main() -> anyhow::Result<()> { .with_env() .with_prom_registry(&prometheus_registry) .init(); - - Ok(run_bridge_node(config).await?.await?) + Ok(run_bridge_node(config, prometheus_registry).await?.await?) } diff --git a/crates/sui-bridge/src/metrics.rs b/crates/sui-bridge/src/metrics.rs new file mode 100644 index 0000000000000..8270a9ba3aa30 --- /dev/null +++ b/crates/sui-bridge/src/metrics.rs @@ -0,0 +1,84 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use prometheus::{ + register_int_counter_vec_with_registry, register_int_counter_with_registry, + register_int_gauge_vec_with_registry, IntCounter, IntCounterVec, IntGaugeVec, Registry, +}; + +#[derive(Clone)] +pub struct BridgeMetrics { + pub(crate) total_err_build_sui_transaction: IntCounter, + pub(crate) total_err_sui_transaction_submission: IntCounter, + pub(crate) total_err_sui_transaction_submission_too_many_failures: IntCounter, + pub(crate) total_err_sui_transaction_execution: IntCounter, + pub(crate) total_requests_received: IntCounterVec, + pub(crate) total_requests_ok: IntCounterVec, + pub(crate) total_requests_error: IntCounterVec, + pub(crate) total_requests_inflight: IntGaugeVec, +} + +impl BridgeMetrics { + pub fn new(registry: &Registry) -> Self { + Self { + total_err_build_sui_transaction: register_int_counter_with_registry!( + "bridge_total_err_build_sui_transaction", + "Total number of errors of building sui transactions", + registry, + ) + .unwrap(), + total_err_sui_transaction_submission: register_int_counter_with_registry!( + "bridge_total_err_sui_transaction_submission", + "Total number of errors of submitting sui transactions", + registry, + ) + .unwrap(), + total_err_sui_transaction_submission_too_many_failures: + register_int_counter_with_registry!( + "bridge_total_err_sui_transaction_submission_too_many_failures", + "Total number of continuous failures to submitting sui transactions", + registry, + ) + .unwrap(), + total_err_sui_transaction_execution: register_int_counter_with_registry!( + "bridge_total_err_sui_transaction_execution", + "Total number of failures of sui transaction execution", + registry, + ) + .unwrap(), + total_requests_received: register_int_counter_vec_with_registry!( + "bridge_total_requests_received", + "Total number of requests received in Server, by request type", + &["type"], + registry, + ) + .unwrap(), + total_requests_ok: register_int_counter_vec_with_registry!( + "bridge_total_requests_ok", + "Total number of ok requests, by request type", + &["type"], + registry, + ) + .unwrap(), + total_requests_error: register_int_counter_vec_with_registry!( + "bridge_total_requests_error", + "Total number of erred requests, by request type", + &["type"], + registry, + ) + .unwrap(), + total_requests_inflight: register_int_gauge_vec_with_registry!( + "bridge_total_requests_inflight", + "Total number of inflight requests, by request type", + &["type"], + registry, + ) + .unwrap(), + } + } + + pub fn new_for_testing() -> Self { + let registry = Registry::new(); + Self::new(®istry) + } +} diff --git a/crates/sui-bridge/src/node.rs b/crates/sui-bridge/src/node.rs index d99118315c549..b44f1a90d5f3e 100644 --- a/crates/sui-bridge/src/node.rs +++ b/crates/sui-bridge/src/node.rs @@ -7,6 +7,7 @@ use crate::{ config::{BridgeClientConfig, BridgeNodeConfig}, eth_syncer::EthSyncer, events::init_all_struct_tags, + metrics::BridgeMetrics, orchestrator::BridgeOrchestrator, server::{handler::BridgeRequestHandler, run_server}, storage::BridgeOrchestratorTables, @@ -27,13 +28,17 @@ use sui_types::{ use tokio::task::JoinHandle; use tracing::info; -pub async fn run_bridge_node(config: BridgeNodeConfig) -> anyhow::Result> { +pub async fn run_bridge_node( + config: BridgeNodeConfig, + prometheus_registry: prometheus::Registry, +) -> anyhow::Result> { init_all_struct_tags(); + let metrics = Arc::new(BridgeMetrics::new(&prometheus_registry)); let (server_config, client_config) = config.validate().await?; // Start Client let _handles = if let Some(client_config) = client_config { - start_client_components(client_config).await + start_client_components(client_config, metrics.clone()).await } else { Ok(vec![]) }?; @@ -51,12 +56,14 @@ pub async fn run_bridge_node(config: BridgeNodeConfig) -> anyhow::Result, ) -> anyhow::Result>> { let store: std::sync::Arc = BridgeOrchestratorTables::new(&client_config.db_path.join("client")); @@ -103,6 +110,7 @@ async fn start_client_components( client_config.key, client_config.sui_address, client_config.gas_object_ref.0, + metrics, ) .await; @@ -186,6 +194,7 @@ fn get_eth_contracts_to_watch( #[cfg(test)] mod tests { use ethers::types::Address as EthAddress; + use prometheus::Registry; use super::*; use crate::config::BridgeNodeConfig; @@ -372,7 +381,7 @@ mod tests { db_path: None, }; // Spawn bridge node in memory - let _handle = run_bridge_node(config).await.unwrap(); + let _handle = run_bridge_node(config, Registry::new()).await.unwrap(); let server_url = format!("http://127.0.0.1:{}", server_listen_port); // Now we expect to see the server to be up and running. @@ -429,7 +438,7 @@ mod tests { db_path: Some(db_path), }; // Spawn bridge node in memory - let _handle = run_bridge_node(config).await.unwrap(); + let _handle = run_bridge_node(config, Registry::new()).await.unwrap(); let server_url = format!("http://127.0.0.1:{}", server_listen_port); // Now we expect to see the server to be up and running. @@ -497,7 +506,7 @@ mod tests { db_path: Some(db_path), }; // Spawn bridge node in memory - let _handle = run_bridge_node(config).await.unwrap(); + let _handle = run_bridge_node(config, Registry::new()).await.unwrap(); let server_url = format!("http://127.0.0.1:{}", server_listen_port); // Now we expect to see the server to be up and running. diff --git a/crates/sui-bridge/src/server/handler.rs b/crates/sui-bridge/src/server/handler.rs index 4fa1eced19229..25221c4cecce0 100644 --- a/crates/sui-bridge/src/server/handler.rs +++ b/crates/sui-bridge/src/server/handler.rs @@ -20,7 +20,6 @@ use sui_types::digests::TransactionDigest; use tap::TapFallible; use tokio::sync::{oneshot, Mutex}; use tracing::info; -use tracing::instrument; use super::governance_verifier::GovernanceVerifier; @@ -256,13 +255,11 @@ impl BridgeRequestHandler { #[async_trait] impl BridgeRequestHandlerTrait for BridgeRequestHandler { - #[instrument(level = "info", skip(self))] async fn handle_eth_tx_hash( &self, tx_hash_hex: String, event_idx: u16, ) -> Result, BridgeError> { - info!("Received handle eth tx request"); let tx_hash = TxHash::from_str(&tx_hash_hex).map_err(|_| BridgeError::InvalidTxHash)?; let (tx, rx) = oneshot::channel(); @@ -276,13 +273,11 @@ impl BridgeRequestHandlerTrait for BridgeRequestHandler { Ok(Json(signed_action)) } - #[instrument(level = "info", skip(self))] async fn handle_sui_tx_digest( &self, tx_digest_base58: String, event_idx: u16, ) -> Result, BridgeError> { - info!("Received handle sui tx request"); let tx_digest = TransactionDigest::from_str(&tx_digest_base58) .map_err(|_e| BridgeError::InvalidTxHash)?; let (tx, rx) = oneshot::channel(); @@ -300,7 +295,6 @@ impl BridgeRequestHandlerTrait for BridgeRequestHandler { &self, action: BridgeAction, ) -> Result, BridgeError> { - info!("Received handle governace action request"); if !action.is_governace_action() { return Err(BridgeError::ActionIsNotGovernanceAction(action)); } diff --git a/crates/sui-bridge/src/server/mock_handler.rs b/crates/sui-bridge/src/server/mock_handler.rs index 553281a568787..8187f14504e8f 100644 --- a/crates/sui-bridge/src/server/mock_handler.rs +++ b/crates/sui-bridge/src/server/mock_handler.rs @@ -13,6 +13,7 @@ use crate::crypto::BridgeAuthorityKeyPair; use crate::crypto::BridgeAuthoritySignInfo; use crate::error::BridgeError; use crate::error::BridgeResult; +use crate::metrics::BridgeMetrics; use crate::types::SignedBridgeAction; use arc_swap::ArcSwap; use async_trait::async_trait; @@ -131,7 +132,12 @@ pub fn run_mock_server( mock_handler: BridgeRequestMockHandler, ) -> tokio::task::JoinHandle<()> { tracing::info!("Starting mock server at {}", socket_address); - let server = axum::Server::bind(&socket_address) - .serve(make_router(Arc::new(mock_handler)).into_make_service()); + let server = axum::Server::bind(&socket_address).serve( + make_router( + Arc::new(mock_handler), + Arc::new(BridgeMetrics::new_for_testing()), + ) + .into_make_service(), + ); tokio::spawn(async move { server.await.unwrap() }) } diff --git a/crates/sui-bridge/src/server/mod.rs b/crates/sui-bridge/src/server/mod.rs index 3e929921d485d..1c7499230cfeb 100644 --- a/crates/sui-bridge/src/server/mod.rs +++ b/crates/sui-bridge/src/server/mod.rs @@ -2,9 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 #![allow(clippy::inconsistent_digit_grouping)] +use crate::with_metrics; use crate::{ crypto::BridgeAuthorityPublicKeyBytes, error::BridgeError, + metrics::BridgeMetrics, server::handler::{BridgeRequestHandler, BridgeRequestHandlerTrait}, types::{ AddTokensOnEvmAction, AddTokensOnSuiAction, AssetPriceUpdateAction, @@ -25,6 +27,7 @@ use fastcrypto::{ use std::sync::Arc; use std::{net::SocketAddr, str::FromStr}; use sui_types::{bridge::BridgeChainId, TypeTag}; +use tracing::{info, instrument}; pub mod governance_verifier; pub mod handler; @@ -56,9 +59,10 @@ pub const ADD_TOKENS_ON_EVM_PATH: &str = pub fn run_server( socket_address: &SocketAddr, handler: BridgeRequestHandler, + metrics: Arc, ) -> tokio::task::JoinHandle<()> { let service = axum::Server::bind(socket_address) - .serve(make_router(Arc::new(handler)).into_make_service()); + .serve(make_router(Arc::new(handler), metrics).into_make_service()); tokio::spawn(async move { service.await.unwrap(); }) @@ -66,6 +70,7 @@ pub fn run_server( pub(crate) fn make_router( handler: Arc, + metrics: Arc, ) -> Router { Router::new() .route("/", get(health_check)) @@ -75,7 +80,7 @@ pub(crate) fn make_router( COMMITTEE_BLOCKLIST_UPDATE_PATH, get(handle_update_committee_blocklist_action), ) - .route(EMERGENCY_BUTTON_PATH, get(handle_emergecny_action)) + .route(EMERGENCY_BUTTON_PATH, get(handle_emergency_action)) .route(LIMIT_UPDATE_PATH, get(handle_limit_update_action)) .route( ASSET_PRICE_UPDATE_PATH, @@ -88,7 +93,7 @@ pub(crate) fn make_router( ) .route(ADD_TOKENS_ON_SUI_PATH, get(handle_add_tokens_on_sui)) .route(ADD_TOKENS_ON_EVM_PATH, get(handle_add_tokens_on_evm)) - .with_state(handler) + .with_state((handler, metrics)) } impl axum::response::IntoResponse for BridgeError { @@ -115,110 +120,164 @@ async fn health_check() -> StatusCode { StatusCode::OK } +#[instrument(level = "error", skip_all, fields(tx_hash_hex=tx_hash_hex, event_idx=event_idx))] async fn handle_eth_tx_hash( Path((tx_hash_hex, event_idx)): Path<(String, u16)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let sig = handler.handle_eth_tx_hash(tx_hash_hex, event_idx).await?; - Ok(sig) + let future = async { + let sig = handler.handle_eth_tx_hash(tx_hash_hex, event_idx).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_eth_tx_hash", future).await } +#[instrument(level = "error", skip_all, fields(tx_digest_base58=tx_digest_base58, event_idx=event_idx))] async fn handle_sui_tx_digest( Path((tx_digest_base58, event_idx)): Path<(String, u16)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let sig: Json = handler - .handle_sui_tx_digest(tx_digest_base58, event_idx) - .await?; - Ok(sig) + let future = async { + let sig: Json = handler + .handle_sui_tx_digest(tx_digest_base58, event_idx) + .await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_sui_tx_digest", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, blocklist_type=blocklist_type, keys=keys))] async fn handle_update_committee_blocklist_action( Path((chain_id, nonce, blocklist_type, keys)): Path<(u8, u64, u8, String)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let blocklist_type = BlocklistType::try_from(blocklist_type).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid blocklist action type: {:?}", err)) - })?; - let blocklisted_members = keys - .split(',') - .map(|s| { - let bytes = Hex::decode(s).map_err(|e| anyhow::anyhow!("{:?}", e))?; - BridgeAuthorityPublicKeyBytes::from_bytes(&bytes) - .map_err(|e| anyhow::anyhow!("{:?}", e)) - }) - .collect::, _>>() - .map_err(|e| BridgeError::InvalidBridgeClientRequest(format!("{:?}", e)))?; - let action = BridgeAction::BlocklistCommitteeAction(BlocklistCommitteeAction { - chain_id, - nonce, - blocklist_type, - blocklisted_members, - }); - - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let blocklist_type = BlocklistType::try_from(blocklist_type).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid blocklist action type: {:?}", + err + )) + })?; + let blocklisted_members = keys + .split(',') + .map(|s| { + let bytes = Hex::decode(s).map_err(|e| anyhow::anyhow!("{:?}", e))?; + BridgeAuthorityPublicKeyBytes::from_bytes(&bytes) + .map_err(|e| anyhow::anyhow!("{:?}", e)) + }) + .collect::, _>>() + .map_err(|e| BridgeError::InvalidBridgeClientRequest(format!("{:?}", e)))?; + let action = BridgeAction::BlocklistCommitteeAction(BlocklistCommitteeAction { + chain_id, + nonce, + blocklist_type, + blocklisted_members, + }); + + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!( + metrics.clone(), + "handle_update_committee_blocklist_action", + future + ) + .await } -async fn handle_emergecny_action( +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, action_type=action_type))] +async fn handle_emergency_action( Path((chain_id, nonce, action_type)): Path<(u8, u64, u8)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let action_type = EmergencyActionType::try_from(action_type).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid emergency action type: {:?}", err)) - })?; - let action = BridgeAction::EmergencyAction(EmergencyAction { - chain_id, - nonce, - action_type, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let action_type = EmergencyActionType::try_from(action_type).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid emergency action type: {:?}", + err + )) + })?; + let action = BridgeAction::EmergencyAction(EmergencyAction { + chain_id, + nonce, + action_type, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_emergency_action", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, sending_chain_id=sending_chain_id, new_usd_limit=new_usd_limit))] async fn handle_limit_update_action( Path((chain_id, nonce, sending_chain_id, new_usd_limit)): Path<(u8, u64, u8, u64)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let sending_chain_id = BridgeChainId::try_from(sending_chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let action = BridgeAction::LimitUpdateAction(LimitUpdateAction { - chain_id, - nonce, - sending_chain_id, - new_usd_limit, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let sending_chain_id = BridgeChainId::try_from(sending_chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let action = BridgeAction::LimitUpdateAction(LimitUpdateAction { + chain_id, + nonce, + sending_chain_id, + new_usd_limit, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_limit_update_action", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, token_id=token_id, new_usd_price=new_usd_price))] async fn handle_asset_price_update_action( Path((chain_id, nonce, token_id, new_usd_price)): Path<(u8, u64, u8, u64)>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let action = BridgeAction::AssetPriceUpdateAction(AssetPriceUpdateAction { - chain_id, - nonce, - token_id, - new_usd_price, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let action = BridgeAction::AssetPriceUpdateAction(AssetPriceUpdateAction { + chain_id, + nonce, + token_id, + new_usd_price, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_asset_price_update_action", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, proxy_address=format!("{:x}", proxy_address), new_impl_address=format!("{:x}", new_impl_address)))] async fn handle_evm_contract_upgrade_with_calldata( Path((chain_id, nonce, proxy_address, new_impl_address, calldata)): Path<( u8, @@ -227,25 +286,41 @@ async fn handle_evm_contract_upgrade_with_calldata( EthAddress, String, )>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let call_data = Hex::decode(&calldata).map_err(|e| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid call data: {:?}", e)) - })?; - let action = BridgeAction::EvmContractUpgradeAction(EvmContractUpgradeAction { - chain_id, - nonce, - proxy_address, - new_impl_address, - call_data, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let call_data = Hex::decode(&calldata).map_err(|e| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid call data: {:?}", e)) + })?; + let action = BridgeAction::EvmContractUpgradeAction(EvmContractUpgradeAction { + chain_id, + nonce, + proxy_address, + new_impl_address, + call_data, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!( + metrics.clone(), + "handle_evm_contract_upgrade_with_calldata", + future + ) + .await } +#[instrument( + level = "error", + skip_all, + fields(chain_id, nonce, proxy_address, new_impl_address) +)] async fn handle_evm_contract_upgrade( Path((chain_id, nonce, proxy_address, new_impl_address)): Path<( u8, @@ -253,22 +328,30 @@ async fn handle_evm_contract_upgrade( EthAddress, EthAddress, )>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - let action = BridgeAction::EvmContractUpgradeAction(EvmContractUpgradeAction { - chain_id, - nonce, - proxy_address, - new_impl_address, - call_data: vec![], - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + let action = BridgeAction::EvmContractUpgradeAction(EvmContractUpgradeAction { + chain_id, + nonce, + proxy_address, + new_impl_address, + call_data: vec![], + }); + let sig: Json = handler.handle_governance_action(action).await?; + + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_evm_contract_upgrade", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, native=native, token_ids=token_ids, token_type_names=token_type_names, token_prices=token_prices))] async fn handle_add_tokens_on_sui( Path((chain_id, nonce, native, token_ids, token_type_names, token_prices)): Path<( u8, @@ -278,67 +361,77 @@ async fn handle_add_tokens_on_sui( String, String, )>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - - if !chain_id.is_sui_chain() { - return Err(BridgeError::InvalidBridgeClientRequest( - "handle_add_tokens_on_sui only expects Sui chain id".to_string(), - )); - } - - let native = match native { - 1 => true, - 0 => false, - _ => { - return Err(BridgeError::InvalidBridgeClientRequest(format!( - "Invalid native flag: {}", - native - ))) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + + if !chain_id.is_sui_chain() { + return Err(BridgeError::InvalidBridgeClientRequest( + "handle_add_tokens_on_sui only expects Sui chain id".to_string(), + )); } - }; - let token_ids = token_ids - .split(',') - .map(|s| { - s.parse::().map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid token id: {:?}", err)) + + let native = match native { + 1 => true, + 0 => false, + _ => { + return Err(BridgeError::InvalidBridgeClientRequest(format!( + "Invalid native flag: {}", + native + ))) + } + }; + let token_ids = token_ids + .split(',') + .map(|s| { + s.parse::().map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid token id: {:?}", err)) + }) }) - }) - .collect::, _>>()?; - let token_type_names = token_type_names - .split(',') - .map(|s| { - TypeTag::from_str(s).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!( - "Invalid token type name: {:?}", - err - )) + .collect::, _>>()?; + let token_type_names = token_type_names + .split(',') + .map(|s| { + TypeTag::from_str(s).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid token type name: {:?}", + err + )) + }) }) - }) - .collect::, _>>()?; - let token_prices = token_prices - .split(',') - .map(|s| { - s.parse::().map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid token price: {:?}", err)) + .collect::, _>>()?; + let token_prices = token_prices + .split(',') + .map(|s| { + s.parse::().map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid token price: {:?}", + err + )) + }) }) - }) - .collect::, _>>()?; - let action = BridgeAction::AddTokensOnSuiAction(AddTokensOnSuiAction { - chain_id, - nonce, - native, - token_ids, - token_type_names, - token_prices, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + .collect::, _>>()?; + let action = BridgeAction::AddTokensOnSuiAction(AddTokensOnSuiAction { + chain_id, + nonce, + native, + token_ids, + token_type_names, + token_prices, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_add_tokens_on_sui", future).await } +#[instrument(level = "error", skip_all, fields(chain_id=chain_id, nonce=nonce, native=native, token_ids=token_ids, token_addresses=token_addresses, token_sui_decimals=token_sui_decimals, token_prices=token_prices))] async fn handle_add_tokens_on_evm( Path((chain_id, nonce, native, token_ids, token_addresses, token_sui_decimals, token_prices)): Path<( u8, @@ -349,73 +442,127 @@ async fn handle_add_tokens_on_evm( String, String, )>, - State(handler): State>, + State((handler, metrics)): State<( + Arc, + Arc, + )>, ) -> Result, BridgeError> { - let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) - })?; - if chain_id.is_sui_chain() { - return Err(BridgeError::InvalidBridgeClientRequest( - "handle_add_tokens_on_evm does not expect Sui chain id".to_string(), - )); - } - - let native = match native { - 1 => true, - 0 => false, - _ => { - return Err(BridgeError::InvalidBridgeClientRequest(format!( - "Invalid native flag: {}", - native - ))) + let future = async { + let chain_id = BridgeChainId::try_from(chain_id).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid chain id: {:?}", err)) + })?; + if chain_id.is_sui_chain() { + return Err(BridgeError::InvalidBridgeClientRequest( + "handle_add_tokens_on_evm does not expect Sui chain id".to_string(), + )); } - }; - let token_ids = token_ids - .split(',') - .map(|s| { - s.parse::().map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid token id: {:?}", err)) + + let native = match native { + 1 => true, + 0 => false, + _ => { + return Err(BridgeError::InvalidBridgeClientRequest(format!( + "Invalid native flag: {}", + native + ))) + } + }; + let token_ids = token_ids + .split(',') + .map(|s| { + s.parse::().map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!("Invalid token id: {:?}", err)) + }) }) - }) - .collect::, _>>()?; - let token_addresses = token_addresses - .split(',') - .map(|s| { - EthAddress::from_str(s).map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid token address: {:?}", err)) + .collect::, _>>()?; + let token_addresses = token_addresses + .split(',') + .map(|s| { + EthAddress::from_str(s).map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid token address: {:?}", + err + )) + }) }) - }) - .collect::, _>>()?; - let token_sui_decimals = token_sui_decimals - .split(',') - .map(|s| { - s.parse::().map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!( - "Invalid token sui decimals: {:?}", - err - )) + .collect::, _>>()?; + let token_sui_decimals = token_sui_decimals + .split(',') + .map(|s| { + s.parse::().map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid token sui decimals: {:?}", + err + )) + }) }) - }) - .collect::, _>>()?; - let token_prices = token_prices - .split(',') - .map(|s| { - s.parse::().map_err(|err| { - BridgeError::InvalidBridgeClientRequest(format!("Invalid token price: {:?}", err)) + .collect::, _>>()?; + let token_prices = token_prices + .split(',') + .map(|s| { + s.parse::().map_err(|err| { + BridgeError::InvalidBridgeClientRequest(format!( + "Invalid token price: {:?}", + err + )) + }) }) - }) - .collect::, _>>()?; - let action = BridgeAction::AddTokensOnEvmAction(AddTokensOnEvmAction { - chain_id, - nonce, - native, - token_ids, - token_addresses, - token_sui_decimals, - token_prices, - }); - let sig: Json = handler.handle_governance_action(action).await?; - Ok(sig) + .collect::, _>>()?; + let action = BridgeAction::AddTokensOnEvmAction(AddTokensOnEvmAction { + chain_id, + nonce, + native, + token_ids, + token_addresses, + token_sui_decimals, + token_prices, + }); + let sig: Json = handler.handle_governance_action(action).await?; + Ok(sig) + }; + with_metrics!(metrics.clone(), "handle_add_tokens_on_evm", future).await +} + +#[macro_export] +macro_rules! with_metrics { + ($metrics:expr, $type_:expr, $func:expr) => { + async move { + info!("Received {} request", $type_); + $metrics + .total_requests_received + .with_label_values(&[$type_]) + .inc(); + $metrics + .total_requests_inflight + .with_label_values(&[$type_]) + .inc(); + + let result = $func.await; + + match &result { + Ok(_) => { + info!("{} request succeeded", $type_); + $metrics + .total_requests_ok + .with_label_values(&[$type_]) + .inc(); + } + Err(e) => { + info!("{} request failed: {:?}", $type_, e); + $metrics + .total_requests_error + .with_label_values(&[$type_]) + .inc(); + } + } + + $metrics + .total_requests_inflight + .with_label_values(&[$type_]) + .dec(); + result + } + }; } #[cfg(test)] From 205b8c53c8e6531cb6ff69d93ec21d2112ebf174 Mon Sep 17 00:00:00 2001 From: Tim Zakian <2895723+tzakian@users.noreply.github.com> Date: Wed, 29 May 2024 17:47:04 -0700 Subject: [PATCH 13/17] [move] Support parsing doc comments before/after/in-between attributes (#17887) ## Description Adds support for parsing doc comments with attributes either before, after, or in the middle of the doc comments. Before, the following were all errors ```move /// Doc comment #[annotation] fun foo() { } /// Doc comment #[annotation] /// Other doc comment fun foo() { } /// Doc comment #[annotation] /// Other doc comment #[annotation] fun foo() { } ``` These are now all supported, and the previous behavior of ```move #[annotation] /// Doc comments fun foo() { } ``` is also maintained. ## Test plan Added additional tests for both the parser, and docgen to make sure we attach doc comments correctly. --- .../crates/move-compiler/src/parser/lexer.rs | 29 +++- .../crates/move-compiler/src/parser/syntax.rs | 20 ++- .../parser/doc_comments_annotations.move | 32 +++++ .../doc_comments_annotations_invalid.exp | 6 + .../doc_comments_annotations_invalid.move | 6 + .../parser/doc_comments_placement.move | 2 +- .../tests/sources/annotation_test.move | 33 +++++ .../sources/annotation_test.spec_inline.md | 125 ++++++++++++++++++ .../annotation_test.spec_inline_no_fold.md | 117 ++++++++++++++++ .../sources/annotation_test.spec_separate.md | 125 ++++++++++++++++++ 10 files changed, 486 insertions(+), 9 deletions(-) create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations.move create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.move create mode 100644 external-crates/move/crates/move-docgen/tests/sources/annotation_test.move create mode 100644 external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline.md create mode 100644 external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline_no_fold.md create mode 100644 external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_separate.md diff --git a/external-crates/move/crates/move-compiler/src/parser/lexer.rs b/external-crates/move/crates/move-compiler/src/parser/lexer.rs index 55a58081feace..320f2cb30d113 100644 --- a/external-crates/move/crates/move-compiler/src/parser/lexer.rs +++ b/external-crates/move/crates/move-compiler/src/parser/lexer.rs @@ -438,6 +438,16 @@ impl<'input> Lexer<'input> { // comments. The documentation comments are not stored in the AST, but can be retrieved by // using the start position of an item as an index into `matched_doc_comments`. pub fn match_doc_comments(&mut self) { + if let Some(comments) = self.read_doc_comments() { + self.attach_doc_comments(comments); + }; + } + + // Matches the doc comments after the last token (or the beginning of the file) to the position + // of the current token. This moves the comments out of `doc_comments` and + // into `matched_doc_comments`. At the end of parsing, if `doc_comments` is not empty, errors + // for stale doc comments will be produced. + pub fn read_doc_comments(&mut self) -> Option { let start = self.previous_end_loc() as u32; let end = self.cur_start as u32; let mut matched = vec![]; @@ -450,10 +460,23 @@ impl<'input> Lexer<'input> { }) .collect::>() .join("\n"); - for span in matched { - self.doc_comments.remove(&span); + if !matched.is_empty() { + for span in matched { + self.doc_comments.remove(&span); + } + Some(merged) + } else { + None } - self.matched_doc_comments.insert(end, merged); + } + + // Calling this function during parsing adds the `doc_comments` to the current location. The + // documentation comments are not stored in the AST, but can be retrieved by using the start + // position of an item as an index into `matched_doc_comments`. + pub fn attach_doc_comments(&mut self, doc_comments: String) { + let attachment_location = self.cur_start as u32; + self.matched_doc_comments + .insert(attachment_location, doc_comments); } // At the end of parsing, checks whether there are any unmatched documentation comments, diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index debfa7bdd661d..f66132780d41f 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -1107,6 +1107,10 @@ fn parse_attribute(context: &mut Context) -> Result> // Parse attributes. Used to annotate a variety of AST nodes // Attributes = ("#" "[" Comma "]")* fn parse_attributes(context: &mut Context) -> Result, Box> { + let mut doc_comments = context + .tokens + .read_doc_comments() + .map_or_else(Vec::new, |doc_comments| vec![doc_comments]); let mut attributes_vec = vec![]; while let Tok::NumSign = context.tokens.peek() { let start_loc = context.tokens.start_loc(); @@ -1125,8 +1129,18 @@ fn parse_attributes(context: &mut Context) -> Result, Box, context: &mut Context, ) -> Result<(ModuleDefinition, Option>), Box> { - context.tokens.match_doc_comments(); let start_loc = context.tokens.start_loc(); let is_spec_module = if context.tokens.peek() == Tok::Spec { @@ -4309,7 +4322,6 @@ fn parse_module_member(context: &mut Context) -> Result { match context.tokens.peek() { // Top-level specification constructs Tok::Invariant => { - context.tokens.match_doc_comments(); let spec_string = consume_spec_string(context)?; consume_token(context.tokens, Tok::Semicolon)?; Ok(ModuleMember::Spec(spec_string)) @@ -4317,7 +4329,6 @@ fn parse_module_member(context: &mut Context) -> Result { Tok::Spec => { match context.tokens.lookahead() { Ok(Tok::Fun) | Ok(Tok::Native) => { - context.tokens.match_doc_comments(); context.tokens.advance()?; // Add an extra check for better error message // if old syntax is used @@ -4342,7 +4353,6 @@ fn parse_module_member(context: &mut Context) -> Result { attributes, context, )?)), _ => { - context.tokens.match_doc_comments(); let start_loc = context.tokens.start_loc(); let modifiers = parse_module_member_modifiers(context)?; let tok = context.tokens.peek(); diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations.move new file mode 100644 index 0000000000000..e66fa265b40f1 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations.move @@ -0,0 +1,32 @@ +/// This is a doc comment above an annotation. +#[allow(unused_const)] +module 0x42::m { + #[allow(dead_code)] + /// This is a doc comment on a constant with an annotation. Below the annotation. + const Error: u32 = 0; + + /// This is a doc comment on a constant with an annotation. Above the annotation. + #[allow(dead_code)] + const OtherError: u32 = 0; + + /// This is the top doc comment + #[allow(dead_code)] + /// This is the middle doc comment + #[ext(something)] + /// This is the bottom doc comment + const Woah: u32 = 0; + + /// This is the top doc comment + #[allow(dead_code)] + /// This is the middle doc comment + #[ext(something)] + const Cool: u32 = 0; + + /// This is a doc comment above a function with an annotation. Above the annotation. + #[allow(dead_code)] + fun test() { } + + #[allow(dead_code)] + /// This is a doc comment above a function with an annotation. Below the annotation. + fun test1() { } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.exp new file mode 100644 index 0000000000000..a52cf92552894 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.exp @@ -0,0 +1,6 @@ +warning[W01004]: invalid documentation comment + ┌─ tests/move_check/parser/doc_comments_annotations_invalid.move:2:5 + │ +2 │ /// Can't have doc comments inside an attribute. + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Documentation comment cannot be matched to a language item + diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.move new file mode 100644 index 0000000000000..0391c56cfd3e5 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_annotations_invalid.move @@ -0,0 +1,6 @@ +#[ext( + /// Can't have doc comments inside an attribute. + something +)] +module 0x42::m { +} diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_placement.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_placement.move index 3f0c62368a937..c5b4035dbfd5f 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_placement.move +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/doc_comments_placement.move @@ -3,7 +3,7 @@ /// Documentation comments can have multiple blocks. /** They may use different limiters. */ module 0x42::M { - /** There can be no doc comment on uses. */ + /** There can be doc comments on uses. */ use std::option::Option; /// This is f. diff --git a/external-crates/move/crates/move-docgen/tests/sources/annotation_test.move b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.move new file mode 100644 index 0000000000000..2aaac100f88d5 --- /dev/null +++ b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.move @@ -0,0 +1,33 @@ +/// This is a doc comment above an annotation. +#[allow(unused_const)] +module 0x42::m { + #[allow(dead_code)] + /// This is a doc comment on a constant with an annotation. Below the annotation. + const Error: u32 = 0; + + /// This is a doc comment on a constant with an annotation. Above the annotation. + #[allow(dead_code)] + const OtherError: u32 = 0; + + /// This is the top doc comment + #[allow(dead_code)] + /// This is the middle doc comment + #[ext(something)] + /// This is the bottom doc comment + const Woah: u32 = 0; + + /// This is the top doc comment + #[allow(dead_code)] + /// This is the middle doc comment + #[ext(something)] + const Cool: u32 = 0; + + /// This is a doc comment above a function with an annotation. Above the annotation. + #[allow(dead_code)] + fun test() { } + + #[allow(dead_code)] + /// This is a doc comment above a function with an annotation. Below the annotation. + fun test1() { } +} + diff --git a/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline.md b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline.md new file mode 100644 index 0000000000000..11143fe356b14 --- /dev/null +++ b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline.md @@ -0,0 +1,125 @@ + + + +# Module `0x42::m` + +This is a doc comment above an annotation. + + +- [Constants](#@Constants_0) +- [Function `test`](#0x42_m_test) +- [Function `test1`](#0x42_m_test1) + + +
+ + + + + +## Constants + + + + +This is the top doc comment +This is the middle doc comment + + +
const Cool: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Below the annotation. + + +
const Error: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Above the annotation. + + +
const OtherError: u32 = 0;
+
+ + + + + +This is the top doc comment +This is the middle doc comment +This is the bottom doc comment + + +
const Woah: u32 = 0;
+
+ + + + + +## Function `test` + +This is a doc comment above a function with an annotation. Above the annotation. + + +
fun test()
+
+ + + +
+Implementation + + +
fun test() { }
+
+ + + +
+ + + +## Function `test1` + +This is a doc comment above a function with an annotation. Below the annotation. + + +
fun test1()
+
+ + + +
+Implementation + + +
fun test1() { }
+
+ + + +
+warning: unused function + ┌─ tests/sources/annotation_test.move:27:9 + │ +27 │ fun test() { } + │ ^^^^ The non-'public', non-'entry' function 'test' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') + +warning: unused function + ┌─ tests/sources/annotation_test.move:31:9 + │ +31 │ fun test1() { } + │ ^^^^^ The non-'public', non-'entry' function 'test1' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') diff --git a/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline_no_fold.md b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline_no_fold.md new file mode 100644 index 0000000000000..d4ce258e9da95 --- /dev/null +++ b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_inline_no_fold.md @@ -0,0 +1,117 @@ + + + +# Module `0x42::m` + +This is a doc comment above an annotation. + + +- [Constants](#@Constants_0) +- [Function `test`](#0x42_m_test) +- [Function `test1`](#0x42_m_test1) + + +
+ + + + + +## Constants + + + + +This is the top doc comment +This is the middle doc comment + + +
const Cool: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Below the annotation. + + +
const Error: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Above the annotation. + + +
const OtherError: u32 = 0;
+
+ + + + + +This is the top doc comment +This is the middle doc comment +This is the bottom doc comment + + +
const Woah: u32 = 0;
+
+ + + + + +## Function `test` + +This is a doc comment above a function with an annotation. Above the annotation. + + +
fun test()
+
+ + + +##### Implementation + + +
fun test() { }
+
+ + + + + +## Function `test1` + +This is a doc comment above a function with an annotation. Below the annotation. + + +
fun test1()
+
+ + + +##### Implementation + + +
fun test1() { }
+
+warning: unused function + ┌─ tests/sources/annotation_test.move:27:9 + │ +27 │ fun test() { } + │ ^^^^ The non-'public', non-'entry' function 'test' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') + +warning: unused function + ┌─ tests/sources/annotation_test.move:31:9 + │ +31 │ fun test1() { } + │ ^^^^^ The non-'public', non-'entry' function 'test1' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') diff --git a/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_separate.md b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_separate.md new file mode 100644 index 0000000000000..11143fe356b14 --- /dev/null +++ b/external-crates/move/crates/move-docgen/tests/sources/annotation_test.spec_separate.md @@ -0,0 +1,125 @@ + + + +# Module `0x42::m` + +This is a doc comment above an annotation. + + +- [Constants](#@Constants_0) +- [Function `test`](#0x42_m_test) +- [Function `test1`](#0x42_m_test1) + + +
+ + + + + +## Constants + + + + +This is the top doc comment +This is the middle doc comment + + +
const Cool: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Below the annotation. + + +
const Error: u32 = 0;
+
+ + + + + +This is a doc comment on a constant with an annotation. Above the annotation. + + +
const OtherError: u32 = 0;
+
+ + + + + +This is the top doc comment +This is the middle doc comment +This is the bottom doc comment + + +
const Woah: u32 = 0;
+
+ + + + + +## Function `test` + +This is a doc comment above a function with an annotation. Above the annotation. + + +
fun test()
+
+ + + +
+Implementation + + +
fun test() { }
+
+ + + +
+ + + +## Function `test1` + +This is a doc comment above a function with an annotation. Below the annotation. + + +
fun test1()
+
+ + + +
+Implementation + + +
fun test1() { }
+
+ + + +
+warning: unused function + ┌─ tests/sources/annotation_test.move:27:9 + │ +27 │ fun test() { } + │ ^^^^ The non-'public', non-'entry' function 'test' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') + +warning: unused function + ┌─ tests/sources/annotation_test.move:31:9 + │ +31 │ fun test1() { } + │ ^^^^^ The non-'public', non-'entry' function 'test1' is never called. Consider removing it. + │ + = This warning can be suppressed with '#[allow(unused_function)]' applied to the 'module' or module member ('const', 'fun', or 'struct') From 078903c7ba71f7d408845d912d4c6dae715345c3 Mon Sep 17 00:00:00 2001 From: Tim Zakian <2895723+tzakian@users.noreply.github.com> Date: Wed, 29 May 2024 18:56:49 -0700 Subject: [PATCH 14/17] [sui-move] Unbreak build (#17985) --- crates/sui-move/src/build.rs | 15 +-------------- crates/sui-move/src/manage_package.rs | 17 +++++++++++++++-- crates/sui-source-validation-service/src/lib.rs | 2 +- crates/sui/src/client_commands.rs | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/sui-move/src/build.rs b/crates/sui-move/src/build.rs index 7180f33d6219e..1b2db3ba96724 100644 --- a/crates/sui-move/src/build.rs +++ b/crates/sui-move/src/build.rs @@ -1,9 +1,9 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::manage_package::resolve_lock_file_path; use clap::Parser; use move_cli::base; -use move_package::source_package::layout::SourcePackageLayout; use move_package::BuildConfig as MoveBuildConfig; use serde_json::json; use std::{fs, path::PathBuf}; @@ -97,16 +97,3 @@ impl Build { Ok(()) } } - -/// Resolve Move.lock file path in package directory (where Move.toml is). -pub fn resolve_lock_file_path( - mut build_config: MoveBuildConfig, - package_path: Option, -) -> Result { - if build_config.lock_file.is_none() { - let package_root = base::reroot_path(package_path)?; - let lock_file_path = package_root.join(SourcePackageLayout::Lock.path()); - build_config.lock_file = Some(lock_file_path); - } - Ok(build_config) -} diff --git a/crates/sui-move/src/manage_package.rs b/crates/sui-move/src/manage_package.rs index aa9e0cddbf33b..9c67611902ae6 100644 --- a/crates/sui-move/src/manage_package.rs +++ b/crates/sui-move/src/manage_package.rs @@ -5,14 +5,14 @@ use anyhow::bail; use clap::Parser; use std::path::PathBuf; +use move_cli::base; use move_package::{ lock_file::{self, LockFile}, + source_package::layout::SourcePackageLayout, BuildConfig, }; use sui_types::base_types::ObjectID; -use crate::build::resolve_lock_file_path; - const NO_LOCK_FILE: &str = "Expected a `Move.lock` file to exist in the package path, \ but none found. Consider running `sui move build` to \ generate the `Move.lock` file in the package directory."; @@ -82,3 +82,16 @@ impl ManagePackage { Ok(()) } } + +/// Resolve Move.lock file path in package directory (where Move.toml is). +pub fn resolve_lock_file_path( + mut build_config: BuildConfig, + package_path: Option, +) -> Result { + if build_config.lock_file.is_none() { + let package_root = base::reroot_path(package_path)?; + let lock_file_path = package_root.join(SourcePackageLayout::Lock.path()); + build_config.lock_file = Some(lock_file_path); + } + Ok(build_config) +} diff --git a/crates/sui-source-validation-service/src/lib.rs b/crates/sui-source-validation-service/src/lib.rs index 83a76179b431b..6eb4218deeaca 100644 --- a/crates/sui-source-validation-service/src/lib.rs +++ b/crates/sui-source-validation-service/src/lib.rs @@ -33,7 +33,7 @@ use url::Url; use move_core_types::account_address::AccountAddress; use move_package::{BuildConfig as MoveBuildConfig, LintFlag}; use move_symbol_pool::Symbol; -use sui_move::build::resolve_lock_file_path; +use sui_move::manage_package::resolve_lock_file_path; use sui_move_build::{BuildConfig, SuiPackageHooks}; use sui_sdk::rpc_types::{SuiTransactionBlockEffects, TransactionFilter}; use sui_sdk::types::base_types::ObjectID; diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index 1ded42100f6c2..4e0670d3627c6 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -32,7 +32,7 @@ use move_package::BuildConfig as MoveBuildConfig; use prometheus::Registry; use serde::Serialize; use serde_json::{json, Value}; -use sui_move::build::resolve_lock_file_path; +use sui_move::manage_package::resolve_lock_file_path; use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion}; use sui_source_validation::{BytecodeSourceVerifier, SourceMode}; From ff78dd084be27abaa408540f3ac17e70da353120 Mon Sep 17 00:00:00 2001 From: mwtian <81660174+mwtian@users.noreply.github.com> Date: Wed, 29 May 2024 21:57:47 -0700 Subject: [PATCH 15/17] Improve sampling of consensus submission latency (#17961) ## Description 1. Do not report latency from sampler until enough samples are taken. This avoids issues with wildly off latency estimates at the start of an epoch. 2. Reduce max submission delay step to 10s, which is still significantly larger than consensus latency. 3. Do not report latency from system transactions that do no require quorum to have formed. These transactions can distort the latency estimate, their submission attempts start before a consensus quorum has formed. 4. Increase expected latency samples to 128. ## Test plan CI Private testnet --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- crates/sui-core/src/consensus_adapter.rs | 55 +++++++++++++++--------- crates/sui-core/src/metrics.rs | 12 ++++-- crates/sui-core/src/overload_monitor.rs | 12 ++++-- 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/crates/sui-core/src/consensus_adapter.rs b/crates/sui-core/src/consensus_adapter.rs index b4e6f16c8c519..587d1521d289d 100644 --- a/crates/sui-core/src/consensus_adapter.rs +++ b/crates/sui-core/src/consensus_adapter.rs @@ -63,10 +63,12 @@ pub mod consensus_tests; const SEQUENCING_CERTIFICATE_LATENCY_SEC_BUCKETS: &[f64] = &[ 0.1, 0.25, 0.5, 0.75, 1., 1.25, 1.5, 1.75, 2., 2.25, 2.5, 2.75, 3., 4., 5., 6., 7., 10., 15., - 20., 25., 30., 60., + 20., 25., 30., 60., 90., 120., 150., 180., 210., 240., 270., 300., ]; -const SEQUENCING_CERTIFICATE_POSITION_BUCKETS: &[f64] = &[0., 1., 2., 3., 5., 10.]; +const SEQUENCING_CERTIFICATE_POSITION_BUCKETS: &[f64] = &[ + 0., 1., 2., 3., 5., 10., 15., 20., 25., 30., 50., 100., 150., 200., +]; pub struct ConsensusAdapterMetrics { // Certificate sequencing metrics @@ -416,14 +418,16 @@ impl ConsensusAdapter { let (position, positions_moved, preceding_disconnected) = self.submission_position(committee, tx_digest); - const MAX_LATENCY: Duration = Duration::from_secs(5 * 60); const DEFAULT_LATENCY: Duration = Duration::from_secs(3); // > p50 consensus latency with global deployment + const MIN_LATENCY: Duration = Duration::from_millis(150); + const MAX_LATENCY: Duration = Duration::from_secs(10); + let latency = self.latency_observer.latency().unwrap_or(DEFAULT_LATENCY); self.metrics .sequencing_estimated_latency .set(latency.as_millis() as i64); - let latency = std::cmp::max(latency, DEFAULT_LATENCY); + let latency = std::cmp::max(latency, MIN_LATENCY); let latency = std::cmp::min(latency, MAX_LATENCY); let latency = latency * 2; let latency = self.override_by_throughput_profiler(position, latency); @@ -719,7 +723,7 @@ impl ConsensusAdapter { let (await_submit, position, positions_moved, preceding_disconnected) = self.await_submit_delay(epoch_store.committee(), &transactions[..]); - let mut guard = InflightDropGuard::acquire(&self, tx_type.to_string()); + let mut guard = InflightDropGuard::acquire(&self, tx_type); let processed_waiter = tokio::select! { // We need to wait for some delay until we submit transaction to the consensus _ = await_submit => Some(processed_waiter), @@ -1022,24 +1026,24 @@ struct InflightDropGuard<'a> { position: Option, positions_moved: Option, preceding_disconnected: Option, - tx_type: String, + tx_type: &'static str, } impl<'a> InflightDropGuard<'a> { - pub fn acquire(adapter: &'a ConsensusAdapter, tx_type: String) -> Self { - let inflight = adapter + pub fn acquire(adapter: &'a ConsensusAdapter, tx_type: &'static str) -> Self { + adapter .num_inflight_transactions .fetch_add(1, Ordering::SeqCst); adapter .metrics - .sequencing_certificate_attempt + .sequencing_certificate_inflight .with_label_values(&[&tx_type]) .inc(); adapter .metrics - .sequencing_certificate_inflight + .sequencing_certificate_attempt .with_label_values(&[&tx_type]) - .set(inflight as i64); + .inc(); Self { adapter, start: Instant::now(), @@ -1053,16 +1057,14 @@ impl<'a> InflightDropGuard<'a> { impl<'a> Drop for InflightDropGuard<'a> { fn drop(&mut self) { - let inflight = self - .adapter + self.adapter .num_inflight_transactions .fetch_sub(1, Ordering::SeqCst); - // Store the latest latency self.adapter .metrics .sequencing_certificate_inflight - .with_label_values(&[&self.tx_type]) - .set(inflight as i64); + .with_label_values(&[self.tx_type]) + .dec(); let position = if let Some(position) = self.position { self.adapter @@ -1089,14 +1091,27 @@ impl<'a> Drop for InflightDropGuard<'a> { }; let latency = self.start.elapsed(); - if self.position == Some(0) { - self.adapter.latency_observer.report(latency); - } self.adapter .metrics .sequencing_certificate_latency - .with_label_values(&[&position, &self.tx_type]) + .with_label_values(&[&position, self.tx_type]) .observe(latency.as_secs_f64()); + + // Only sample latency after consensus quorum is up. Otherwise, the wait for consensus + // quorum at the beginning of an epoch can distort the sampled latencies. + // Technically there are more system transaction types that can be included in samples + // after the first consensus commit, but this set of types should be enough. + if self.position == Some(0) { + // Transaction types below require quorum existed in the current epoch. + // TODO: refactor tx_type to enum. + let sampled = matches!( + self.tx_type, + "shared_certificate" | "owned_certificate" | "checkpoint_signature" | "soft_bundle" + ); + if sampled { + self.adapter.latency_observer.report(latency); + } + } } } diff --git a/crates/sui-core/src/metrics.rs b/crates/sui-core/src/metrics.rs index 54cc6fbf9de4a..d9093e32b9ae3 100644 --- a/crates/sui-core/src/metrics.rs +++ b/crates/sui-core/src/metrics.rs @@ -29,13 +29,17 @@ impl LatencyObserver { } pub fn report(&self, latency: Duration) { - const MAX_SAMPLES: usize = 64; + const EXPECTED_SAMPLES: usize = 128; let mut data = self.data.lock(); data.points.push_back(latency); data.sum += latency; - if data.points.len() >= MAX_SAMPLES { + if data.points.len() < EXPECTED_SAMPLES { + // Do not initialize average latency until there are enough samples. + return; + } + while data.points.len() > EXPECTED_SAMPLES { let pop = data.points.pop_front().expect("data vector is not empty"); - data.sum -= pop; // This does not overflow because of how running sum is calculated + data.sum -= pop; // This does not underflow because of how running sum is calculated } let latency = data.sum.as_millis() as u64 / data.points.len() as u64; self.latency_ms.store(latency, Ordering::Relaxed); @@ -44,7 +48,7 @@ impl LatencyObserver { pub fn latency(&self) -> Option { let latency = self.latency_ms.load(Ordering::Relaxed); if latency == u64::MAX { - // Not initialized yet (0 data points) + // Not initialized yet (not enough data points) None } else { Some(Duration::from_millis(latency)) diff --git a/crates/sui-core/src/overload_monitor.rs b/crates/sui-core/src/overload_monitor.rs index f02df06ac54fa..b162880226844 100644 --- a/crates/sui-core/src/overload_monitor.rs +++ b/crates/sui-core/src/overload_monitor.rs @@ -424,12 +424,16 @@ mod tests { .build() .await; + // Initialize latency reporter. + for _ in 0..1000 { + state + .metrics + .execution_queueing_latency + .report(Duration::from_secs(20)); + } + // Creates a simple case to see if authority state overload_info can be updated // correctly by check_authority_overload. - state - .metrics - .execution_queueing_latency - .report(Duration::from_secs(20)); let authority = Arc::downgrade(&state); assert!(check_authority_overload(&authority, &config)); assert!(state.overload_info.is_overload.load(Ordering::Relaxed)); From 117c0e2e78a9dad936d2d0d1be9d0d21191a005d Mon Sep 17 00:00:00 2001 From: mwtian <81660174+mwtian@users.noreply.github.com> Date: Wed, 29 May 2024 22:37:28 -0700 Subject: [PATCH 16/17] [CI] reduce log verbosity in tests (#17990) ## Description Sometimes failed integration tests can produce too much logs and overwhelm the browser even when reading raw logs. ## Test plan CI --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .github/workflows/narwhal.yml | 2 ++ .github/workflows/rust.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/narwhal.yml b/.github/workflows/narwhal.yml index 05830497339a5..320af4674794d 100644 --- a/.github/workflows/narwhal.yml +++ b/.github/workflows/narwhal.yml @@ -49,6 +49,8 @@ env: RUSTUP_MAX_RETRIES: 10 # Don't emit giant backtraces in the CI logs. RUST_BACKTRACE: short + # Some integration tests can produce too much INFO logs that are infeasible to be printed on failure. + RUST_LOG: error # RUSTFLAGS: -D warnings RUSTDOCFLAGS: -D warnings diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f1b77c9e65f38..c23c2ba166f58 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -36,6 +36,8 @@ env: RUSTUP_MAX_RETRIES: 10 # Don't emit giant backtraces in the CI logs. RUST_BACKTRACE: short + # Some integration tests can produce too much INFO logs that are infeasible to be printed on failure. + RUST_LOG: error # RUSTFLAGS: -D warnings RUSTDOCFLAGS: -D warnings From d44f1c04d88ec464c02eba4641ea0abf6cbe18bd Mon Sep 17 00:00:00 2001 From: Tim Zakian <2895723+tzakian@users.noreply.github.com> Date: Wed, 29 May 2024 23:01:52 -0700 Subject: [PATCH 17/17] [move] Fix and add interactive disassembler (#17986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes the interactive disassembler, and adds the flag for it to `sui move disassemble` (with the `-i` flag). See screencast for what this looks like. https://github.com/MystenLabs/sui/assets/2895723/50a75eb1-3d84-4c00-a30b-ab051c2ca169 ## Test plan 👀 --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [X] CLI: Adds new `-i` flag to `sui move disassemble` that creates an interactive disassembled bytecode-to-source explorer. - [ ] Rust SDK: --- crates/sui-move/src/disassemble.rs | 5 ++++- .../move/crates/move-bytecode-viewer/src/bytecode_viewer.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/sui-move/src/disassemble.rs b/crates/sui-move/src/disassemble.rs index 877ea40cbf63e..f962a5e3c09c4 100644 --- a/crates/sui-move/src/disassemble.rs +++ b/crates/sui-move/src/disassemble.rs @@ -22,6 +22,9 @@ pub struct Disassemble { /// Whether to display the disassembly in raw Debug format #[clap(long = "Xdebug")] debug: bool, + + #[clap(short = 'i', long = "interactive")] + interactive: bool, } impl Disassemble { @@ -40,7 +43,7 @@ impl Disassemble { .expect("Cannot convert module name to string") .to_owned(); move_cli::base::disassemble::Disassemble { - interactive: false, + interactive: self.interactive, package_name: None, module_or_script_name: module_name, debug: self.debug, diff --git a/external-crates/move/crates/move-bytecode-viewer/src/bytecode_viewer.rs b/external-crates/move/crates/move-bytecode-viewer/src/bytecode_viewer.rs index 8669d82dd156b..3a7092bd8a551 100644 --- a/external-crates/move/crates/move-bytecode-viewer/src/bytecode_viewer.rs +++ b/external-crates/move/crates/move-bytecode-viewer/src/bytecode_viewer.rs @@ -47,7 +47,10 @@ impl<'a> BytecodeViewer<'a> { fn build_mapping(&mut self) { let regex = Regex::new(r"^(\d+):.*").unwrap(); - let fun_regex = Regex::new(r"^public\s+([a-zA-Z_]+)\(").unwrap(); + let fun_regex = + Regex::new(r"^(?:public(?:\(\w+\))?|native|entry)?\s*(\w+)\s*(?:<.*>)?\s*\(.*\).*\{") + .unwrap(); + let mut current_fun = None; let mut current_fdef_idx = None; let mut line_map = HashMap::new();