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

Rename authorize to approve #534

Merged
merged 4 commits into from
Nov 9, 2022
Merged

Conversation

MartinMinkov
Copy link
Contributor

We rename 'authorize' to approve with the aim to take advantage of the concept of 'approve' in Ethereum. We hope that developers will intuitively understand what it means to 'approve' an AccountUpdate given that a smart contract in Ethereum must get approval to spend tokens.

We rename 'authorize' to approve in the aims to take advantage of the
concept of 'approve' in Ethereum. We hope that developers will
intuitively understand what it means to 'approve' a AccountUpdate given
that a smart contract in Ethereum must get approval to spend tokens.
@MartinMinkov MartinMinkov force-pushed the feat/rename-authorize-to-approve branch from 0ca3df3 to d89e71d Compare November 8, 2022 18:55
@MartinMinkov MartinMinkov self-assigned this Nov 8, 2022
@MartinMinkov MartinMinkov changed the title Rename authorize to `approve Rename authorize to approve Nov 8, 2022
Copy link
Collaborator

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for updating the doc comments as well

CHANGELOG.md Outdated
@@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- BREAKING CHANGE: `tx.send()` is now asynchronous: old: `send(): TransactionId` new: `send(): Promise<TransactionId>` and `tx.send()` now directly waits for the network response, as opposed to `tx.send().wait()`
- `Circuit.witness` can now be called outside circuits, where it will just directly return the callback result
- BREAKING CHANGE: Static methods of type `.fromString()`, `.fromNumber()` and `.fromBigInt()` on `Field`, `UInt64`, `UInt32` and `Int64` are not longer supported.
- Rename `SmartContract.experimental.authorize()` to `SmartContract.experimental.approve()`
Copy link
Collaborator

Choose a reason for hiding this comment

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

authorize wasn't released yet - it's in the "Added" section! could you remove this and change the name in the "Added" section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

@MartinMinkov MartinMinkov force-pushed the feat/rename-authorize-to-approve branch from d89e71d to 3dbf5ca Compare November 8, 2022 21:49
@MartinMinkov MartinMinkov force-pushed the feat/rename-authorize-to-approve branch from 3dbf5ca to 39529c1 Compare November 9, 2022 01:05
@MartinMinkov MartinMinkov merged commit e151439 into main Nov 9, 2022
@MartinMinkov MartinMinkov deleted the feat/rename-authorize-to-approve branch November 9, 2022 19:10
MartinMinkov added a commit that referenced this pull request Nov 15, 2022
* changelog

* proper link

* make ./run handle files with internal imports

* make AccountUpdate a valid method argument

* add `this.authorize` to adopt update & witness its children

* restore old ./run behaviour if the new one fails

* fix token example

* use struct

* remove redundant function

* tweak run script again

* wip  callback refactor

* fixup

* update bindings

* Fetch Account's verification key.

* make tx.send async

* add error trace

* helpers for pretty-printing updates

* update changelog

* make tx.send async in all examples and tests

* More details on what to check when a state fetch fails

* fix token test

* tweak witness to just get & return fields

* clone updates when witnessing, so prover doesn't interfere with tx
-> to still be able to identify the same update, give them an id

* fixes

* avert future name clash

* make unit tests rely on existing build
(so that Mina CI can be independent of checked-in bindings)

* merge accountUpdateFromCallback into authorize

* changelog

* make child layout spec more verbose and understandable

* update bindings

* fix returning values in the top-level transactoin case

* reenable having no authorization when calling zkapps

* Dex Application - supplyLiquidity Happy Path

* Use predefined keys for debugging purposes

* Add getBalance function to TokenContract

* Refactor supplyLiquidity methods to remove return type and use getBalance

* Update dex run script to use supplyLiquidity methods

* add enableProofs

* add isDelegateCall and  compute `caller` from it

* fix logic to tompute caller in the prover

* set delegate call in authorize, clone

* wip get dex running

* wip add isDelegateCall to circuit

* caller progress

* label acount updates

* no delegate call from the top level

* make token send go through

* Add failure cases for supply liquidity in DEX

* Add proof verification into the tests.

* Add proof verification into the tests.

* Fix for tests (Proof verification).

* Skipping some tests in CI condition refactoring.

* Commenting out txn signing to allow proving..

* add error for long token symbol & make it a struct

* fix dependency cycle

* make isDelegateCall a Bool

* remove caller hacks from dex/run

* remove unnecessary code

* remove debugging

* fix jest tests

* make SequenceEvents exportable for external use

* delay children hashing until caller is known, fix logic to know when we're at the top level

* update bindings

* instantiate subclass of struct, not base class

* Remove clone for lazyAuthorization

* Update dex example

* better struct fix

* fix dummy proof logic

* fixes to dex contract logic & balance sum

* minor

* handle more cases when making account update a child

* try setting some zkapps to delegate_call, but creates different token owner issues

* typos

* make redeem liquidity work

* minor

* Simplify things by utilizing test fixtures.

* enable changing proofsEnabled

* dex debugging with proofs

* make Circuit.witness create a new unchecked snark context, and only call `exists` in checked mode

* fix assertion about hashed child

* any children

* fix demo

* make tx sending async

* temp fix

* fix integration test

* add verification key test

* minor

* allow children which can't delegate token spending power, as default for authorize

* wip swap happy path

* missing access permission issue repro

* fix dummy proof for child updates

* fix remaining bugs

* fix tests

* remove comment

* swap fix

* refine the logic for when `exists` is called

* Circuit.log

* stop implicitly attaching account updates to tx

* remove debugging

* remove unused z token

* changelog

* remove redundant checks guarding Circuit.witness

* fix precondition tests

* changelog

* fix attach logic

* add edgecase with proof verification

* rm unused code

* fix broken permission and vk tests

* manually specify feepayer nonce

* dont fetch nonce if nonce is specified

* address feedback

* add Uint.toBigInt

* helper to check token balances

* add checks for supply liquidity (happy path)

* checks for redeem liquidity (no vesting)

* fix nonce calculation

* start adding timing, but missing error on repeated liquidity supplying

* pretty printing improvements

* expose account timing

* use ES private class fields instead of `private`

* roll back one private field bc its not inherited

* better interface for setting local global slot

* add vesting

* finish redeem liquidity tests

* swap add checks for happy path

* working on overflow tests

* Add optional assertion failure messages to SnarkyJS methods (#470)

* Add optional error message to Field.assertEquals

* Add optional message for Bool.assertEquals

* Add optional message for Group.assertEquals

* Add optional message to Field.assert compares

* Add optional message to Field.assertBoolean

* Add optional message to Bool.assertTrue/False

* Changelog

* feat: Add optional message for UInt64 assert methods

* feat: Add optional message for UInt32 assert methods

* feat: Add optional message for Int64 assert methods

* Update bindings

* merge in existing supply failures and add the rest

* bonus test for the supplying after lock period

* run tests with and w/o vesting

* fix uint comparisons

* push fix to examples

* add unit test for division in the prover

* fix second run by recreating ledger

* update bindings

* add back setGlobalSlot

* add back setGlobalSlot

* Add args to the callback method invocation (#516)

* Add args to the callback method invocation

* Changelog

* remove static from methods

* remove static definitions for field

* update changelog

* fix internal usage

* start snarky doc comments

* Removes static one/zero/-1 from Field

Part of #511

* Deprecates instead of removes Field members

* Add shutdown to end of primitive unit test (#525)

* Add timeout minutes to github action (#526)

* dump initial mina-signer experiments

* type check mina-signer

* make transaction-helpers generic over base types

* remove most specializations to base types away

* make transaction-helpers fully generic

* lift customTypes to functor level

* rename

* change dir and filenames

* remove undefined from typemap

* remove primitive types from typemap

* autogenerate type map

* move everything to common folder

* update bindings

* fix tests

* Update int.test.ts

* fix tests

* redo merge conflict fix

* Add additional token tests (#522)

* Refactor token tests and add additional tests

* Add comment descriptions

* bump node CI time

* Rename `authorize` to `approve` (#534)

* Rename 'authorize' to 'approve'

We rename 'authorize' to approve in the aims to take advantage of the
concept of 'approve' in Ethereum. We hope that developers will
intuitively understand what it means to 'approve' a AccountUpdate given
that a smart contract in Ethereum must get approval to spend tokens.

* Changelog

* Update tokens test

* Make `JSONValue` internally used only (#536)

* Make JSONValue internally used only

Move the definition of JSONValue into circuit_value.ts since there are
many types that depend on JSONValue inside that file. We also removed
any usages of JSONValue from public fromJSON methods since that type is
intended for internal use.

* Replace JSONValue with any type

* Fix tests

Update int.test.ts

fix tests

redo merge conflict fix

 Fix tests

* Removed all uses of JSONValue with any

* cache nonce

* minor

* make fetch tests run with CI && import from fetch

* make tests abort

* more tests

* add nested escaped string

* add init and show opt-in proving for it in example

* fix init logic so that we can assure its only run once

* tweak error

* fix token test

* better type for numberOfRuns

* swap import order

* adjust tests

* fix dex

* changelog

* fix error parsing logic

* replace links

* enable manually triggering a workflow

* fix yaml

* move Reducer and MerkleStuff off Experimental

* update examples

* move token and approve off this.experimental

* update examples

* changelog

* fix merkle tree tests

* fix token test

* deprecate this.sign and document requireSignature

* changelog

* changelog

* more comments

* more comments

* 0.7.0

* changelog

Co-authored-by: Serhii Shymkiv <sergey@shimkiv.com>
Co-authored-by: Florian <florian@technotro.com>
Co-authored-by: Brandon Kase <brandon.kase@gmail.com>
Co-authored-by: Martin Minkov <minkovlmartin@gmail.com>
Co-authored-by: Comdex <wcomdex@foxmail.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.

2 participants