Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TS SDK v2] Add AccountAddress #9564

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Conversation

banool
Copy link
Contributor

@banool banool commented Aug 9, 2023

Description

This PR implements AccountAddress for the v2 TS SDK. This implementation conforms to AIP-40.

Test Plan

cd ecosystem/typescript/sdk_v2
npx jest -- tests/unit/account_address.test.ts

@banool banool force-pushed the banool/ts-sdk-v2-account-address branch from 257b050 to 0f7f377 Compare August 9, 2023 00:19
Copy link
Contributor

@0xmaayan 0xmaayan left a comment

Choose a reason for hiding this comment

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

Looks good! left some comments

export type Uint64 = bigint;
export type Uint128 = bigint;
export type Uint256 = bigint;
export type AnyNumber = bigint | number;
Copy link
Contributor

Choose a reason for hiding this comment

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

requireLeadingZeroX?: boolean;
};

// TODO: As part of reviewing this we should discuss these options. I'm partially
Copy link
Contributor

Choose a reason for hiding this comment

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

I think too many options can be confusing, and tbh I am a bit confused with all those options lol. if the standard is

 * - LONG: 64 characters, with or without a leading 0x.
 * - SHORT: Some number of characters as a suffix with padding zeroes if necessary, with or without a leading 0x.

why we need all those options and why one would want to requireLeadingZeroX? just have this class/method handles with or without leading 0x.

Copy link
Contributor Author

@banool banool Aug 10, 2023

Choose a reason for hiding this comment

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

That's a fair point. So here's what I want to update the standard to:


Functions that parse / validate strings as AccountAddress MUST accept the following:

a. LONG
b. SHORT for special addresses

The following correspond to representations actively in circulation. Implementations MAY accept these formats for the sake of backwards compatibility with the existing ecosystem.

c. LONG with or without leading 0x
d. SHORT with or without leading 0x for all addresses (not just special)

These are allowed for the sake of migration, since some tools return account addresses in these latter formats and cannot be updated without breaking backwards compatibility. Where possible, developers should avoid accepting these formats (e.g. when building APIs, wallets, tools, etc) but it is allowed in order to maintain compatibility.


So the idea is we should make only a and b accepted by default but users can opt into more relaxed parsing if necessary.

The alternative is we make the parsing relaxed by default and the dapp dev can opt into stricter parsing.

I'd rather go stricter by default in this case, since this is a whole new v2 SDK, so it's a good time to make larger changes like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We spoke offline, I'll make a change to the AIP.

ecosystem/typescript/sdk_v2/src/types/account_address.ts Outdated Show resolved Hide resolved
*
* @returns True if the string is valid, false if not.
*/
static isValid(str: string, validityOptions: ValidityOptions = defaultValidityOptions): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not combine this and isValidWithReason?

Copy link
Contributor Author

@banool banool Aug 10, 2023

Choose a reason for hiding this comment

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

You think we should just remove the one that returns only a boolean? I suppose that's a good idea, then it's up to the user which of the values they use in the response. I will make a single function called isValid that returns both the boolean + string.

}

/**
* Returns whether an address is special, where special is defined as 0x0 to 0xf
Copy link
Contributor

Choose a reason for hiding this comment

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

could we explain a bit more what is a special address? maybe some examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could yeah, though what do you think about linking to the AIP instead, where it explains it in detail?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes we can. maybe something like "special address is reserved/used by aptos framework" and then a link to it

ecosystem/typescript/sdk_v2/src/types/account_address.ts Outdated Show resolved Hide resolved
ecosystem/typescript/sdk_v2/src/types/account_address.ts Outdated Show resolved Hide resolved

// These options influence fromStr and the isValid* functions. You may use them to make
// the parsing of those functions strictier. Not setting these options is the best way
// to comply with AIP 40 but while migrating it may be helpful to be able to enforce
Copy link
Contributor

Choose a reason for hiding this comment

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

in what cases we would want to be able to enforce stricter behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss this here: #9564 (comment).

// This is the internal representation of an account address.
readonly address: Bytes;

static ADDRESS_ONE: AccountAddress = AccountAddress.fromStr("0x1");
Copy link
Contributor

Choose a reason for hiding this comment

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

what are these for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already had ADDRESS_ONE (under a different name) in the previous AccountAddress class. We tend to deploy all framework code on 0x1, 0x2, 0x3, or 0x4. So it's handy to have constants for those. We do the same thing in Rust and Python.

Copy link
Contributor

@xbtmatt xbtmatt Aug 11, 2023

Choose a reason for hiding this comment

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

Yes, I use the 0x1 a lot, so much that I have it memorized lol

TxnBuilderTypes.AccountAddress.CORE_CODE_ADDRESS

I like greg's suggestion of AccountAddress.ONE etc

@banool banool force-pushed the banool/ts-sdk-v2-account-address branch 3 times, most recently from 826d355 to 2874b23 Compare August 10, 2023 19:48
@banool banool marked this pull request as ready for review August 10, 2023 19:48
@banool banool force-pushed the banool/ts-sdk-v2-account-address branch from 2874b23 to 15268f1 Compare August 10, 2023 19:52
@@ -0,0 +1,317 @@
// Copyright © Aptos Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

following this comment on HexData class, lets move this class to either utils or code folder https://github.com/aptos-labs/aptos-core/pull/9595/files#r1290573135

@banool banool changed the base branch from main to banool/ts-sdk-v2-hex August 10, 2023 20:03
@banool banool force-pushed the banool/ts-sdk-v2-hex branch 10 times, most recently from 20ced9a to 45ee6ff Compare August 10, 2023 23:20
@banool banool force-pushed the banool/ts-sdk-v2-account-address branch from 15268f1 to c55c3ec Compare August 10, 2023 23:42
@banool banool force-pushed the banool/ts-sdk-v2-hex branch 3 times, most recently from 8df56a8 to 8d3e207 Compare August 10, 2023 23:59
@banool banool force-pushed the banool/ts-sdk-v2-account-address branch from c55c3ec to fc44dfa Compare August 11, 2023 00:00
Copy link
Contributor

@gregnazario gregnazario left a comment

Choose a reason for hiding this comment

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

I'm not sure you wanted to include BCS, but I think if we do, we should have some preset tests to ensure serialization / deserialization works properly (I don't know if I missed it)

Comment on lines 10 to 12
export const MAX_U64_BIG_INT: Uint64 = BigInt(2 ** 64) - BigInt(1);
export const MAX_U128_BIG_INT: Uint128 = BigInt(2 ** 128) - BigInt(1);
export const MAX_U256_BIG_INT: Uint256 = BigInt(2 ** 256) - BigInt(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious... does BigInt(2**64) work if, it's above the value for a standard number?

I just realized this (we use this in the other SDK too right?)

* ```ts
* const deserializer = new Deserializer(new Uint8Array([24, 0xc3, 0xa7, 0xc3, 0xa5, 0xe2, 0x88, 0x9e,
* 0xe2, 0x89, 0xa0, 0xc2, 0xa2, 0xc3, 0xb5, 0xc3, 0x9f, 0xe2, 0x88, 0x82, 0xc6, 0x92, 0xe2, 0x88, 0xab]));
* assert(deserializer.deserializeStr() === "çå∞≠¢õß∂ƒ∫");
Copy link
Contributor

Choose a reason for hiding this comment

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

Lol, these bytes, maybe a more human readable example?

Copy link
Contributor

Choose a reason for hiding this comment

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

Lol yea this was copied from the example in the original sdk idk why it was this string...

whenever we do do this we should use something like this:

const deserializer = new Deserializer(new Uint8Array([6, 0x61, 0x62, 0x63, 0x31, 0x32, 0x33]));
assert(deserializer.deserializeStr() == "abc123");

}

private ensureBufferWillHandleSize(bytes: number) {
while (this.buffer.byteLength < this.offset + bytes) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: same comparison as way above, but inverted condition :)

Comment on lines 96 to 99
serializeBool(value: boolean): void {
if (typeof value !== "boolean") {
throw new Error("Value needs to be a boolean");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting that this is validated in boolean, but not in the others?

Oh, I guess there are checks in checkNumberRange

Comment on lines 125 to 144
@checkNumberRange(0, MAX_U16_NUMBER)
serializeU16(value: Uint16): void {
this.serializeWithFunction(DataView.prototype.setUint16, 2, value);
}

/**
* Serializes a uint32 number.
*
* BCS layout for "uint32": Four bytes. Binary format in little-endian representation.
* @example
* ```ts
* const serializer = new Serializer();
* serializer.serializeU32(305419896);
* assert(serializer.getBytes() === new Uint8Array([0x78, 0x56, 0x34, 0x12]));
* ```
*/
@checkNumberRange(0, MAX_U32_NUMBER)
serializeU32(value: Uint32): void {
this.serializeWithFunction(DataView.prototype.setUint32, 4, value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

These already handle little endianness automatically?

Comment on lines 53 to 59
static ADDRESS_ONE: AccountAddress = AccountAddress.fromString({ str: "0x1" });

static ADDRESS_TWO: AccountAddress = AccountAddress.fromString({ str: "0x2" });

static ADDRESS_THREE: AccountAddress = AccountAddress.fromString({ str: "0x3" });

static ADDRESS_FOUR: AccountAddress = AccountAddress.fromString({ str: "0x4" });
Copy link
Contributor

Choose a reason for hiding this comment

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

I forget can statics be used in Typescript like AccountAddress.ONE, that may be easier then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call let me do that.

*/
constructor(args: { data: Uint8Array }) {
if (args.data.length !== AccountAddress.LENGTH) {
throw new ParsingError("Expected address of length 32", AddressInvalidReason.INCORRECT_NUMBER_OF_BYTES);
Copy link
Contributor

Choose a reason for hiding this comment

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

32 bytes

});

// These tests show that fromString only parses addresses with a leading 0x and only
// SHORT if it is a specil address.
Copy link
Contributor

Choose a reason for hiding this comment

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

*special

@banool
Copy link
Contributor Author

banool commented Aug 11, 2023

It'll probably be cleaner if I remove the serde functions from AccountAddress and remove the BCS stuff from this PR. Then we can add the BCS stuff later. I'll do that tomorrow.

Base automatically changed from banool/ts-sdk-v2-hex to main August 11, 2023 15:31
@banool banool force-pushed the banool/ts-sdk-v2-account-address branch 2 times, most recently from f9312e1 to 78d2ec5 Compare August 11, 2023 15:50
Copy link
Contributor

@0xmaayan 0xmaayan left a comment

Choose a reason for hiding this comment

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

LGTM, left come comments

}

// ===
// Methods for creating an instance of Hex from other types.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Methods for creating an instance of Hex from other types.
// Methods for creating an instance of AccountAddress from other types.

longWithout0x: "0000000000000000000000000000000000000000000000000000000000000010",
};

const ADDRESS_OTHER: Addresses = {
Copy link
Contributor

Choose a reason for hiding this comment

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

could we also add tests for fromHexInput and fromHexInputRelaxed ?

@banool banool force-pushed the banool/ts-sdk-v2-account-address branch from 78d2ec5 to e827a83 Compare August 11, 2023 18:15
@banool
Copy link
Contributor Author

banool commented Aug 11, 2023

I just added tests for fromHexInput and fromHexInputRelaxed. I also added a toUint8Array method (like we have with Hex) and added tests for that.

@banool banool enabled auto-merge (squash) August 11, 2023 18:25
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732

Compatibility test results for aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732 (PR)
1. Check liveness of validators at old version: aptos-node-v1.5.1
compatibility::simple-validator-upgrade::liveness-check : committed: 4471 txn/s, latency: 7257 ms, (p50: 7500 ms, p90: 10300 ms, p99: 13200 ms), latency samples: 169900
2. Upgrading first Validator to new version: e827a8383ad7211699566499847dd4fddac65732
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1789 txn/s, latency: 16252 ms, (p50: 18900 ms, p90: 22300 ms, p99: 22600 ms), latency samples: 93060
3. Upgrading rest of first batch to new version: e827a8383ad7211699566499847dd4fddac65732
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1877 txn/s, latency: 15555 ms, (p50: 19500 ms, p90: 21900 ms, p99: 22300 ms), latency samples: 92000
4. upgrading second batch to new version: e827a8383ad7211699566499847dd4fddac65732
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2329 txn/s, latency: 9592 ms, (p50: 10000 ms, p90: 13800 ms, p99: 14200 ms), latency samples: 137440
5. check swarm health
Compatibility test for aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732 passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on e827a8383ad7211699566499847dd4fddac65732

two traffics test: inner traffic : committed: 6496 txn/s, latency: 6026 ms, (p50: 5400 ms, p90: 8100 ms, p99: 14000 ms), latency samples: 2813140
two traffics test : committed: 100 txn/s, latency: 3107 ms, (p50: 2900 ms, p90: 3500 ms, p99: 11700 ms), latency samples: 1860
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.243, avg: 0.212", "QsPosToProposal: max: 0.487, avg: 0.189", "ConsensusProposalToOrdered: max: 0.651, avg: 0.603", "ConsensusOrderedToCommit: max: 0.529, avg: 0.506", "ConsensusProposalToCommit: max: 1.157, avg: 1.109"]
Max round gap was 1 [limit 4] at version 1286430. Max no progress secs was 3.702172 [limit 10] at version 1286430.
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732

Compatibility test results for aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732 (PR)
Upgrade the nodes to version: e827a8383ad7211699566499847dd4fddac65732
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2924 txn/s, latency: 7416 ms, (p50: 7800 ms, p90: 10400 ms, p99: 11400 ms), latency samples: 163760
5. check swarm health
Compatibility test for aptos-node-v1.5.1 ==> e827a8383ad7211699566499847dd4fddac65732 passed
Test Ok

@banool banool merged commit ff3688a into main Aug 11, 2023
72 of 73 checks passed
@banool banool deleted the banool/ts-sdk-v2-account-address branch August 11, 2023 23:28
aalok-t pushed a commit to aalok-thakkar/aptos-core that referenced this pull request Aug 13, 2023
xbtmatt pushed a commit that referenced this pull request Aug 13, 2023
vusirikala added a commit that referenced this pull request Aug 23, 2023
* [storage] support sharding in fast_sync

handle sharded kv batches

handle sharded merkel tree

save progress

* [TS SDK] Support network and API endpoints on `AptosConfig` (#9549)

* support network and api urls on AptosConfig

* devnet as default network

* Remove tutorial links from descriptions

Tutorials are linked on tutorial title and again on the tutorial description just next to the title. Small nitpick, but is slightly confusing when you just arrive to the dev docs, since you expect that second link to be something different.

* add profiling crate with cpu and memory profiling

fix comments and warnings

fix comments

resolve comments

add crate to cargo.toml

add additional functions for different cases of profiling

fix cargo.toml

* remove unnecessary dependency

* fix lint errors

* fix lint errors

* fix lint error in cargo.toml

* [testsuite][pangu][pte] Creating a Pangu Command to Create a Transaction Emitter (#9495)

* [testsuite][pangu][pte] Initial work for the transaction emitter.

* [testsuite][pangu][pte] Finished the transaction emitter comand.

* Update testsuite/test_framework/kubernetes.py

good catch!

Co-authored-by: Balaji Arun <balaji@aptoslabs.com>

* [testsuite][pangu][pte] Addressed Balaji's comments.

* [testsuite][pangu][pte] Hotfix for Forge compatibility.

* [testsuite][pangu][pte] Another hotfix.

* [testsuite][pangu][pte] Added support for dry runs, and fixed a bug in get testnet

* [testsuite][pangu][pte] Minor changes.

* Update README.md

---------

Co-authored-by: Olsen Budanur <olsenbudanur@Olsens-MBP.localdomain>
Co-authored-by: Balaji Arun <balaji@aptoslabs.com>

* replay-verify: log skipped version when it happens

* [dashboards] sync grafana dashboards (#9440)

Co-authored-by: rustielin <rustielin@users.noreply.github.com>

* [forge stable] Fix HAProxy test - increase latency limits (#9575)

* [indexer grpc] reduce the data service streaming channel size (#9590)

* [refactoring] Remove unused genesis check from StateView (#9589)

`is_genesis()` was defined in `StateView` but never used apart
from a single test with `MockVM`. But the test set it to false
always...

Removing it to have better view interfaces.

* [TS SDK] Use `account_transactions` and `account_transaction_aggregate` queries (#9403)

* improve indexer client to support tokenv2 sorting and new queries

* use account_transactions query

* Optional  argument on  is not positional

* [Dev Docs] Add small note about target-version in backup docs.

* swich to gcp

* add semgrep for github workflows (#9522)

add semgrep for GitHub workflows

* [GHA] Run smoke and performance tests when appropriate.

* [Spec] Ensures of transaction_validation (#9461)

* hp_trans_valid

* hp_trans_validation

* pre-pr

* fix trim

---------

Co-authored-by: chan-bing <zzywullr@gmail.com>

* [GHA] Small tweaks to jobs.

* [TS SDK] filter amount > 0 on getTokenOwnersData and include all query fields (#9593)

* filter amount > 0 on getTokenOwnersData

* export all query fields

* update changelog

* [Python SDK] Add a token_transfer function to the python SDK (#9422)

* Adding transfer_token function to aptos_token_client.py and adding documentation tags for the new your_first_nft tutorial.

* Fixed strange formatting

* Created a transfer_object function in RestClient class and altered the transfer_token function in the AptosTokenClient to just call transfer_object. Left it in for ease of use

* [deps] Update clap dependency to be consistent across crates (#9605)

* Fix node compatibility test CI (#9609)

* [gas] fix abstract gas cost for storage (#9501)

* [forge] Add latency for different workloads (#9415)

* remove redundant test (#9613)

* [dag] Handle certified node in DAG Driver (#9312)

* [dag] Handle certified node in dag driver
* [dag] Trigger new round on handler start
* [dag] Use only the aptos_time_service crate

* [NFT Metadata Crawler] Fix error for defaulting to original raw_image_uri (#9611)

* Fix error for defaulting to original raw_image_uri

* move db commit outside

* [ts-sdk example] Adding an example for rotation offer capability and signer offer capability with signed structs (#9425)

* Adding the offer capability example. Uses signed structs in typescript.

* remove unnecessary aptosClient from getAccount call

Co-authored-by: Maayan <maayan@aptoslabs.com>

* Fixing network URL unnecessary code and potential unalignment between network/faucet url

* Shortening # hyphens in output string

* Moving the chainId to the bottom of the struct list since it's potentially undefined, and explaining in the sign struct function that the proof bytes must be in that specific order.

* Cleaning up the code and making it more readable

* Formatting

* Use a much cleaner and reusable serializable class instead of a struct

---------

Co-authored-by: Maayan <maayan@aptoslabs.com>

* bump version to 1.18.0 (#9607)

* [compiler-v2] Use livevar analysis to optimize file format generation (#9361)

* [compiler-v2] Use livevar analysis to optimize file format generation

This connects the live-variable analysis which is already present in the Move prover to the v2 bytecode pipeline. This information is then used in the file-format generator to make better decisions when to move or copy values.

Livevar is a standard compiler construction data flow analysis. It computes the set of variables which are alive (being used in subsequent reachable code) before and after each program point. The implementation from the prover uses our dataflow analysis framework and is [here](https://github.com/aptos-labs/aptos-core/blob/206f529c0c9d8488e27d2e50297178f0caf429a5/third_party/move/move-prover/bytecode/src/livevar_analysis.rs#L381). In this PR we create the first processing step in the bytecode pipeline with the new module `pipeline/livevar_analysis_step.rs` which forwards logical work to the existing analysis.

* Addressing reviewer comments.

* [storage] update resource requriement

* [TS SDK v2] Add Hex and HexInput types (#9595)

Co-authored-by: maayan <maayansavir@gmail.com>

* [cleanup] Remove foreign contracts (#9623)

* [Spec] Ensures of reconfiguration.move (#9383)

* init

* hp

* hp reconfig

* init

* fix comment

* fix comment

---------

Co-authored-by: chan-bing <zzywullr@gmail.com>

* Revert "Revert "[NFT Metadata Crawler] Dockerize Parser (#9541)"" (#9622)

This reverts commit aa81b95.

* Clearer comments in `resource_account.move` (#9555)

* Explicitize that the txn_seq_num param on the epilogue is unused

With this we can go through all historical txns to make sure.

* [dag] Integrate Order Rule with Dag Driver (#9452)

* [gas] convert output to InternalGas cost (#9603)

* [move-ir-compiler] add Nop token support (#9599)

* [dag] Integrate Dag Fetcher with Dag Driver (#9453)

* [dag] Integrate Dag Fetcher with Dag Driver
* [dag] Handle fetch response in dag handler

* [TS SDK v2] Add AccountAddress (#9564)

* [CLI] Disallow "rotating" to same private key (#9546)

* trivial: fix gas meter mutability

* Remove unused deps for CLI (#9531)

* typo in functions.md

* typo spec-lang.md

typo

* Update package-upgrades.md

space added

* Update developer-docs-site/docs/move/book/package-upgrades.md

* Update unit-testing.md

typo

* [GHA] Only run all perf tests on schedule.

* [TS SDK example] Add a working example of converting a MultiEd25519 account to a MultiSig account (#9516)

* An example of converting a MultiEd25519 account to a MultiSig account

Cleaned up the file even more, added assertions and print statements at the end

Removing incorrect addresses

Formatting and clarifying the steps. Changing network to DEVNET from local

Removing confusing comment from prior version and adding comment about making sure secondsTilExpiration is set accordingly

Fixing file name

Fixing variable name

Making header comments less long

* Adding the ability to add metadata to the multisig account, since we were using empty arrays before. Asserted that the values on-chain match the input values at the end

* Using a much cleaner and reusable serializable class for the signed message

* Removing the unnecessary multisig as sender option

* Update e2e flow description at top of file

* Resolving rebase conflict and simplifying the signature construction

* Clarify the process of creating a MultiEd25519 account and that public keys are different from addresses

* Improve clarity of comments on token.move

* [NFT Metadata Crawler] Fix raw image uri parse error (#9628)

* Fix raw_image_uri parse bug

* upsert

* update compatibility test base

* [Dev Docs] Add note about indexer bootstrap support.

* Update developer-docs-site/docs/nodes/indexer-fullnode.md

* [dashboards] sync grafana dashboards

* remove payer from StateValueMetadata

* Track slot allocation fee on state metadata

* enable state value metadata tracking for devnet

* [compiler v2] Fix bugs around compilation of vector code (#9636)

* [compiler v2] Fix bugs around compilation of vector code

This fixes a few bugs and closes #9629

- Usage of precompiled stdlib in transactional tests: v2 compiler currently (and probably never) supports precompiled modules, working around this
- Stack-based bytecode generator created wrong target on generic function calls
- Needed to make implicit conversion from `&mut` to `&` explicit in generated bytecode via Freeze operation

Added some additional tests while debugging this.

* Adding a new option `--print-bytecode` which can be provided to the `//# publish` and `//# run` transactional test runner command. This is the applied (for now) to the `sorter` test case only. Also introduced logic to map well-known vector functions to the associated builtin opcodes.

* [TS SDK v2] Reject invalid SHORT form for special addresses in fromString (#9640)

* [TS SDK v2] Reject invalid SHORT form for special addresses in fromString

* Update account_address.ts

* Update account_address.ts

* [TS SDK v2] Add equality methods for Hex and AccountAddress (#9641)

* Handle empty network address in CLI (#9576)

* [CLI][e2e] Tests create-resource-account and derive-resource-account-address (#8757)

* CLI e2e for resource account

* add account list to test resource account

* [prover] lock version dependency for z3 and boogie (#8718) (#9524)

Co-authored-by: Aalok Thakkar <aalok@Aaloks-MacBook-Pro.local>

* Update workflows (#9650)

* Update semgrep.yaml to also run daily

* update semgrep rule

* fix workflows

* Update .github/workflows/semgrep.yaml

Co-authored-by: Balaji Arun <balaji@aptoslabs.com>

---------

Co-authored-by: Balaji Arun <balaji@aptoslabs.com>

* [dag] bootstrap logic (#9455)

* add guard field to struct

* temp

* add executor benchmark profiling

* small fixes

* lint fix

* lint fix

* lint fix

* lint fix

* lint fix

* update cargo.lock

* [Spec] Ensures of resources.move (#9382)

* init

* fix md

* fix comment

* [Spec] Ensures of transaction_fee (#9460)

* hp trans_fee

* pre-pr

* add the condition that proposer == @vm_reserved

* fix comment

---------

Co-authored-by: chan-bing <zzywullr@gmail.com>

* [TS SDK v2] Remove redundant code in AccountAddress.fromString (#9662)

* [docs] Rename Aptos Move CLI -> Aptos CLI (#9655)

* Thoroughly fix Issue 8875: Error out in compilation when encounter an unknown attribute (#9229)

Fix issue-8875 thoroughly: warning for unknown attributes, add --skip-attribute-check flag.  Omit warning on aptos_std library files, to avoid warning on some existing code.

A few tangential fixes:

* [third party/.../test, compiler] Fix Move.toml files to point to local files instead of network to avoid tests using old stdlib.  Oddly, this requires fixes to capitalization of std.  Fix the resolution warning to make it clear that std needs to be uncapitalized.

* [compiler] Hack to move-compiler to avoid warning about unknown attributes in aptos_std=0x1.
This avoids surprising warning on currently deployed library code.

* [compiler] test cases and stdlib source code updates for attributes checks PR

* [aptos stdlib] Now that we have verified that the move-compiler Aptos stdlib hack works, fix stdlib to use #[test_only] on test helper functions instead of #[testonly].  Leave struct attributes since it's a deployment problem to make it disappear.

* [Storage Service] Add type support for subscription requests.

* [Storage Service] Add subscription stream support.

* [Storage Service] Adopt subscription metadata and loop when serving
subscriptions.

* Make AccountAddress from_str conform to AIP-40, add from_str_strict (#9186)

* [TS SDK v2] Remove redundant code in AccountAddress.fromString

* Make AccountAddress from_str conform to AIP-40

* Use thiserror for AccountAddressParseError

* [Executor-benchmark] Make connected grps of txns fully conflicting grps (#9647)

[Executor-benchmark] Make connected grps of txns fully conflicting grps

Fully conflicting grps are more meaningful pattern for testing out the sharded
executor because they cannot be executed parallely (that is what we intend to benchmark).

Cmd:
cargo run -p aptos-executor-benchmark -- --block-size 100 --connected-tx-grps 5 --shuffle-connected-txns run-executor --main-signer-accounts 1000 --data-dir /tmp/some-db --checkpoint-dir /tmp/some-checkpoint --blocks 2

For a good random workload we would want 'num_accounts_per_grp' to be much greater than 'num_txns_per_grp'.
The command enforces 'num_signer_accounts' > '2 * num_txns_per_grp'.

* [NFT Metadata Crawler] Stop parsing completely if token_uri has already been parsed (#9649)

* stop parsing completely if token_uri has already been parsed, add logs, add more panics (failed to write to gcs, failed to commit postgres tx)

* add comment to clarify

* error -> warn

* rename

* Fixed signer to be formatted properly (#8866)

* Fixed signer to be formatted properly

- fixed the issue in `string_utils`
- replaced `testonly` with `test_only`

* Add a feature flag

* [Quorum Store] A couple metrics to observe backpressure better (#9239)

### Description

A couple metrics to observe backpressure better

* [Execution] Do not hold the Arc of committed block in execution. (#9674)

* [NFT Metadata Crawler] Increase HTTP request for large files (#9677)

* retry time increase

* add to constants file

* remove unneeded comment

* Revert "swich to gcp"

This reverts commit 8f28f93.

* [NFT Metadata Crawler] Maintain image aspect ration on resize (#9688)

* maintain image aspect ration on resize

* fix lint

* [Spec] update spec for vesting.move (#9115)

* new vesting

* new vesting

* init

* fix comment

* fix md

* add comment

* add head

* add func total_accumulated_rewards time

* rm boogie

* fixed timeout

* close timeout function's verification

* rust lint fix

* del head

---------

Co-authored-by: chan-bing <zzywullr@gmail.com>

* [event_v2] module event extension, attribute and extended check

* [eventv2] make v0/v1 to v1/v2

* [event_v2] test

* [events v2] Sample of how to check for event type attributes in the extended checker

* [eventv2] add module publishing validation

* [eventv2] add module event feature

* [eventv2] tests and fixes

* [eventv2] address comments and forbid script emitting module events

* [eventv2] add script event verifier to forbid event emitting

* add #[event] to known attributes

* trivial: add required feature to dependency to fix tests

`e2e-move-tests` when running alone doesn't pass because
`pub const GAS_UNIT_PRICE: u64 = 0;` is not in effect

* Revert "add #[event] to known attributes"

This reverts commit 5af30fb.

* Revert "[eventv2] add script event verifier to forbid event emitting"

This reverts commit 6a30bcd.

* Revert "[eventv2] address comments and forbid script emitting module events"

This reverts commit e4a9c2c.

* Revert "[eventv2] tests and fixes"

This reverts commit 4040b3d.

* Revert "[eventv2] add module event feature"

This reverts commit 5a12afc.

* Revert "[eventv2] add module publishing validation"

This reverts commit 7543f6e.

* Revert "[events v2] Sample of how to check for event type attributes in the extended checker"

This reverts commit 72cb7b4.

* Revert "[event_v2] test"

This reverts commit c93e61d.

* Revert "[eventv2] make v0/v1 to v1/v2"

This reverts commit 0ba554c.

* Revert "[event_v2] module event extension, attribute and extended check"

This reverts commit ed99bf0.

* [compiler v2] Move the stackless bytecode crate out of prover (#9698)

This moves the `move-stackless-bytecode` crate out of its current location in the `move-prover` into the `move-model`. This is pure relocation. Subsequent PRs will split off prover specific code and move it back to the prover tree.

* [gas][docs] example guides (#9633)

* [refactoring] Relax trait bounds, reduce dependence on `StateView` (#9644)

API only needs move resolver, and not state view. Decoupling
`MoveResolverExt` into `AptosMoveResolver + StateView` so
that any future changes were not visible on API side.

1. Move API checks for resource groups where it should be.

2. Make trait bounds for config not require state view. Right now
state view is used all over the place although it seems this can
be avoided.

3. Reduce the usage of `MoveResolverExt`: e.g. session only
needs to talk to resolver.

4. State value metadata has its own resolver. This makes storage
accesses almost uniform (still need to fix `read_write_set`) which
should be implemented in storage adapter.

5. Some minor typo fixes and dead code removal.

* [dag] network sender implementation (#9456)

* [Storage Service] Add subscription stream support.

* [Storage Service] Add tests for subscription streams.

* [Storage Service] Add tests for stream looping.

* Add get_state_value_u128 method to TStateView (#9097)

* [Spec] Ensures for voting (#9462)

* new_voting

* run lint

* little change

* fix md

* fix schema name

* fix error in simplemap

---------

Co-authored-by: chan-bing <zzywullr@gmail.com>

* [scripts] fixes to arch linux (pacman)

* Archlinux / python / pip insist that you use the package manager
  solutions not python for pip
* The `libudev-dev` seems to exist in the default system install and under
  a wildly different name.

* [Block Executor] Refactor view & output traits & mvhashmap (#9659)

* [MVHashMap] convenient (shared) view state, aggregator v1 port

* [Block executor] new traits for processing the output

* [typo*] aptos-token and wallets (#9661)

* Update aptos-token.md

typo "maxium"/maximum

* [typo*]Update wallets.md

dot added

* [compiler v2] Stackless Bytecode Refactoring (#9713)

This is the 2nd step in the refactoring started in #9698.  This splits off prover specific parts from the crate `move-model/bytecode` into `move-prover/bytecode-pipeline`. Sharable dataflow analysis and transformation processors, like livevar and reaching definitions, stay. The tests have been split as well, and a common testing driver has been moved in its own test utility crate.

Github does not nicely show diffs like this, but this is functionally a no-op which only Moves code around.

* [DAG] Adding randomness field in dag node (#9687)

* [NFT Metadata Crawler] Fix double slash URI parsing error, add ACKing PubSub message on receive (#9706)

* fix double slash uri parsing error, acking on message receive

* edit var names

* fix test

* [api] Add consistent API testing (#9577)

* [api] Add consistent API testing (#9146)

This commit introduces consistent API testing where we run a consistent load through the network mimicking various user flows. See proposal for more high level information.

Changes:

Created aptos-api-tester crate which runs 4 user flows:
Account creation
Coin transfer
NFT transfer
Module publishing, and outputs results to stdout.
Added token-client to the SDK with the methods needed to run Your First NFT.
mirroring the typescript SDK and the rust coin client by using existing builders
Added PartialEq to aptos_rest_client::Account for testing.
Added the crate to the tools docker image to run with GCP Cloud Run.

* [api] Add metrics to api tester (#9307)

This commit introduces logging tools on top of the consistent API testing introduced here. See proposal for more high level information.

Changes:

Added metrics pusher to aptos-api-tester with 4 histogram metrics for success, error, fail rates, and latency.
Added logs to tests instead of printing to stdout.

* [api] Add threads and timestamps to api tests (#9349)

This commit builds threads and run ID on top of the consistent API testing introduced here. See proposal for more high level information.

Changes:

Refactored tests into dedicated file tests.rs.
Switched to creating accounts for every tests instead of using the same and created pre test set up routines in testsetups.rs.
Added run ID start_time to metrics.

* [api] Add support for strictness level for consistency on API tests (#9444)

Description
This commit builds threads and run ID on top of the consistent API testing introduced here. See proposal for more high level information.

One problem we saw in the dashboard is that the problems we observed were regarding eventual consistency. We added support for adjusting the strictness level for such errors.

Changes:

Refactor of tests into tests module and decomposing into steps (setting up for next PR, individual step timing).
Persistent checks for information retrieval.
Added token support for Testnet faucet.

* [api] Add individual step timing to api tester (#9502)

This commit builds individual step timing on top of the consistent API testing introduced here. See proposal for more high level information.

This addition allows us to dive deeper on what the issue is should we detect any flow is consistently slower.

Changes:

Add timer macro in macros.rs.
Add helpers for emitting metrics and refactor process_result.
Put publish_module in its own thread by creating a tokio runtime.
Add sleeps between persistent checks.
Reduce API client sleep time to 100ms from 500ms.
Make test setups persistent.

* [api] Add view function testing to the API tester (#9658)

This commit adds a new test which tests a simple view function.

* [dag] e2e integration test (#9457)

* [event_v2] module event extension, attribute and extended check

* [eventv2] make v0/v1 to v1/v2

* [event_v2] test

* [events v2] Sample of how to check for event type attributes in the extended checker

* [eventv2] add module publishing validation

* [eventv2] add module event feature

* [eventv2] tests and fixes

* [eventv2] address comments and forbid script emitting module events

* [eventv2] add script event verifier to forbid event emitting

* add #[event] to known attributes

* [eventv2] api test

* Update staking-pool-operations.md (#8685)

* Update staking-pool-operations.md

corrected CLI command to create staking_contract

previously the command created a direct pool type

* Update staking-pool-operations.md

add additional 8 0s

* Update Docker images (#9699)

Co-authored-by: gedigi <gedigi@users.noreply.github.com>

* [smart_vector] add destroy function for dropable T (#9434)

* [CLI] Add support for setting faucet auth token (#9715)

* [CLI] Add support for setting faucet auth token

* Use FaucetClient

* Unify thread name, and combine all threads in the same thread pool in framegraph produced by cpu_profiler. (#9721)

* Feat/pypi ci (#9512)

* add python sdk publish ci

* remove check version

* [framework] Reset the release yaml for upcoming 1.7 release

Test Plan: ran yq / generate proposals locally

```
target/performance/aptos-release-builder generate-proposals --release-config aptos-move/aptos-release-builder/data/release.yaml --output-dir output
```

---------

Co-authored-by: Bo Wu <bo@aptoslabs.com>
Co-authored-by: Maayan <maayan@aptoslabs.com>
Co-authored-by: Álvaro Lillo Igualada <alilloig@gmail.com>
Co-authored-by: yunuseozer <yunusozer@berkeley.edu>
Co-authored-by: Oğuzhan (Olsen) Budanur <74462406+olsenbudanur@users.noreply.github.com>
Co-authored-by: Olsen Budanur <olsenbudanur@Olsens-MBP.localdomain>
Co-authored-by: Balaji Arun <balaji@aptoslabs.com>
Co-authored-by: aldenhu <msmouse@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: rustielin <rustielin@users.noreply.github.com>
Co-authored-by: igor-aptos <110557261+igor-aptos@users.noreply.github.com>
Co-authored-by: larry-aptos <112209412+larry-aptos@users.noreply.github.com>
Co-authored-by: George Mitenkov <georgemitenk0v@gmail.com>
Co-authored-by: Josh Lind <josh.lind@hotmail.com>
Co-authored-by: Gerardo Di Giacomo <gerardo@aptoslabs.com>
Co-authored-by: Zorrot Chen <UI_Zorrot@163.com>
Co-authored-by: chan-bing <zzywullr@gmail.com>
Co-authored-by: Matt <90358481+xbtmatt@users.noreply.github.com>
Co-authored-by: Greg Nazario <greg@gnazar.io>
Co-authored-by: Daniel Porteous (dport) <daniel@dport.me>
Co-authored-by: Victor Gao <10379359+vgao1996@users.noreply.github.com>
Co-authored-by: Justin Chang <37165464+just-in-chang@users.noreply.github.com>
Co-authored-by: Wolfgang Grieskamp <wg@aptoslabs.com>
Co-authored-by: maayan <maayansavir@gmail.com>
Co-authored-by: Alin Tomescu <tomescu.alin@gmail.com>
Co-authored-by: William Law <williamlaw.wtl@gmail.com>
Co-authored-by: Vladislav ~ cryptomolot <88001005+cryptomolot@users.noreply.github.com>
Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>
Co-authored-by: JasperTimm <jaspa@jaspa.codes>
Co-authored-by: Jin <128556004+0xjinn@users.noreply.github.com>
Co-authored-by: aalok-t <140445856+aalok-t@users.noreply.github.com>
Co-authored-by: Aalok Thakkar <aalok@Aaloks-MacBook-Pro.local>
Co-authored-by: Brian R. Murphy <132495859+brmataptos@users.noreply.github.com>
Co-authored-by: Manu Dhundi <manudhundi@gmail.com>
Co-authored-by: Junkil Park <jpark@aptoslabs.com>
Co-authored-by: Brian (Sunghoon) Cho <brian@aptoslabs.com>
Co-authored-by: Guoteng Rao <3603304+grao1991@users.noreply.github.com>
Co-authored-by: Bo Wu <bowu365@gmail.com>
Co-authored-by: Aaron Gao <lightmark@gmail.com>
Co-authored-by: Andrei Tonkikh <andrei.tonkikh@gmail.com>
Co-authored-by: Rati Gelashvili <gelash@users.noreply.github.com>
Co-authored-by: Daniel Xiang <66756900+danielxiangzl@users.noreply.github.com>
Co-authored-by: Nurullah Giray Kuru <giraykuru@gmail.com>
Co-authored-by: michelle-aptos <120680608+michelle-aptos@users.noreply.github.com>
Co-authored-by: gedigi <gedigi@users.noreply.github.com>
Co-authored-by: Fenix <zhuzhenfeng1993@hotmail.com>
Co-authored-by: Perry Randall <perryjrandall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants