convex-dev / convex Goto Github PK

Should be possible to execute multiple statements in a block:

{
  a();
  1+2;
  f(a);
}

Which should translate to something like:

(do
  (a)
  (+ 1 2)
  (f a))

Probably needs rules:

• Block which contains zero or more statements
• a StatementSeparator that matches a semicolon and optional spacing.

Will need to disambiguate from a map literal. Normal maps will have at one or more (odd number) commas as separators, which should be clear. {} might be tricky.

@kroezone what do you have in mind for the syntax? Here's a Python-ish syntax:

Testing whether an inexistant account is an actor results in a NullPointerException which breaks the calling account.

Possible cause: no null checking after retrieving an AccountStatus, before calling .isActor on it.

Related to #53 , I have a hard time understanding how the Reader escapes/unescapes things. For example, trying these in the sandbox (as is) or directly evaling on the CVM (manually escaping since talking to the JVM):

"\\"  ;; Fine
"\""  ;; Fine
"\b" ;; Syntax error
"\n" ;; Syntax error

Either I am confused or there is something to clarify ;)

Since maps can be coerced to vectors, they should be coercible to sets (matching Clojure behavior by the way). Currently

(vec {:a :b})  ;; => [[:a :b]]

(set {:a :b}) ;; => Cast error

Currently, the Reader accepts doubles that start with a dot:

(= .5 0.5)

While this is acceptable, it creates pathological cases with symbols since they accept dots as valid characters:

'a.5  ;; => Symbol
'+.5  ;; => Double, 0.5

What should happen with:

(long 0xFF) ;; 255?
(long 0x1000) ;; 4096>
(long 0xFFFFFFFFFFFFFFFF) ;; -1?
(long 0xFFFFFFFFFFFFFFFFFF) ;; argument error?

Behaviour is currently undefined, seems to be an issue.

Currently we just have a simple import macro:

(import some-address :as alias)

Should extend this and consider:

• Ability to make multiple imports in one statement?
• Ability to directly refer to specific symbols (like Clojure's refer)
• Ability to replace the convex.core default bindings
• Additional security checks?

That was surprising but yes, filter is indeed missing in Core. For a language relying so much on functional programming, it should be added.

Edit: And similarly, filterv if a difference is ultimately implemented between map and mapv.

Currently Strings are encoded directly as embedded objects.

We need a way to cleanly separate:

• Short Strings that can be embedded (perhaps up to X bytes or Y chars)
• Medium Strings that must be represented as Cells
• Long Strings that require a representation as multiple cells (similar to Blobs)

Perhaps needs a custom AString type that implements CharSequence

When it comes to quoting and unquoting, the fact that the litteral notation for NaN is a regular symbol creates that kind of inconsistent behavior:

'[NaN]      ;; => Actually, NaN is a symbol
'[~NaN]    ;; => Ok, NaN is NaN
'#{~NaN}  ;; => For some reason #{(unquote NaN)}

Intuitively, I don't think you can solve this at the level of quoting/unquoting. It might be that NaN and +-Infinity (in the future) should be expressed through a dedicated notation. For instance, Clojure uses ##NaN.

PROBLEM: When writing EDN, Blob outputs its content as a number in hexadecimal notation, meaning that the EDN reader interprets it as a long or a bigint (eg. Clojure's BigInt). While it somewhat works, it is highly uncomfortable. For instance, Clojure's BigInt do not support bitwise operations and any meaningful work becomes tedious.

FIX:

a) Output an actual hexstring, a proper string. Currently prevented by #53. Not perfect but arguably more amenable than a large number.

b) Output a vector of individual bytes. Technically accurate but can be slow for bigger data (especially in Clojure). On the other hand, data stored on chain should not be huge.

Either from the sandbox or directly on the CVM:

(fn ([]) ([a]))

;; => (fn (fn [] nil) (fn [a] nil))

Result is an invalid form, evaluating it produces:

Compile: fn instance requires a vector of parameters but got form: (fn [] nil)

The following destructuring breaks the executing account:

This is due to how the CVM currently handles what we could call "non-terminal &":

(let [[a & b c d e] [1 2 3 4 5 6 7 8]] [a b c d e])

;; => [1 [2 3 4 5] 6 7 8]

This is really tricky, so tricky that it is illegal in Clojure which only tolerates & in a terminal position (ie. [... & binding]).

Keywords do not support namespaces but providing one in the literal notation (akin to symbols) produces this weird behavor:

(= :foo/bar
    :bar)
    
;; true

Probable cause: According to Mike, the Reader reuses the symbol implementation for keywords and openly discards the namespace.

Operations like those fail in Clojure which is arguable more coherent. The fact that chars are somewhat numbers look more a leaky abstraction leading to counter-intuitive results (unless one is accustomed to C).

(+ \a 42)  ;; => 139

(inc \a)  ;; => 98

(max \ù 42)  ;; => ù

Both in literal notation and by using keyword or symbols, anything more than 32 chars is not understood by the CVM (syntax error and cast error respectively).

(count "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")   ;;  => 33

(symbol "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")  ;; => Cast error

'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa  ;; => Syntax error

Is it a design decision?

We should have a custom if statement that mirror Java / JS

if (condition) {
  dosomething();
}

Suggest getting code blocks working first.

In Clojure, lists are not associative and do not provide random access, unlike vectors.

In Convex, what lists are and their difference from vectors is a bit confusing since you can use assoc and get.

(assoc (list :a :b) 0 :OK)  ;; => '(:OK :b)

(get (list :a :b) 1)  ;; => :b

Furthermore, it muddles the algorithmic difference between get and nth, where get should be about an optimal random access whereas nth is supposedly linear (converting any data structure to a sequence (logical list) and hoping from cell to cell).

It should be possible for function application to be applied repeatedly, so that a function can be returned from another function and immediately applied.

f(a)(b)

Which should be interpreted as ((f a) b) in Convex Lisp.

May require a rule with ZeroOrMore(FunctionParameters) or similar, similar to how we handle InfixExtension.

There is a potential exploit with Memory Accounting if the Account records are not fixed size:

1. Execute some setup code such that:
2. Memory accounting causes at least one Account size to increase
3. Execute some more code to make the Account size decrease (e.g. a transfer)
4. Enjoy a memory refund
5. Sell the memory for a profit
6. Repeat

Might not be practical (the transaction of triggering the refund of 1-2 bytes may not be economically viable), but still a risk to plug. BEst solution is probably just to make balance and allowance 64-bit

With new embedding changes, there is a risk that some of the large composite objects could have a message size that exceeds the maximum, which would cause many nasty problems

We need to validate the maximum encoding size for every object type.

Suggested strategy:

• Create sample objects of maximum size
• Create static final constants listing the expected maximum size
• Ensure sample object equal expected maximum
• Add tests for invariants relating size of composite objects to size of embedded objects

Currently NFT actor seems to accept transfers of NFTs to itself:

(import convex.nft-tokens :as nft)

(call nft (create-token nil nil))
 => 0
(call nft (create-token nil nil))
 => 1
(call nft (create-token nil nil))
 => 2

(asset/balance nft *address*)
 => #{1,2,0}

(asset/transfer nft [nft #{1 2}] nil)

(asset/balance nft *address*)
 => #{0}

Would expect the transfer (penultimate line) to fail?

In a low-level language like C, booleans are simply integers and can behave as such. However, in a high-level language, this should be unexpected for the sake of type safety. Even Java agrees.

So I strongly argue that the following should be illegal:

(number? true)  ;; => true

(inc true)  ;; => 2

Looking at the implementation of CVMBool.longValue, it looks like it has been designed on purpose. Any rationale?

The CVM will require special handling for non-embedded Strings that exceed a certain length (similar to Blob / BlobTree).

Need to implement:

• Cut-off size beyond which Strings cease to be embedded
• Cell-based wrapper for long strings
• Update runtime functions to understand both String types (can probably both implement CharSequence for covenience)

Currently convex.asset does:

  (defn balance
    ^{:doc {:description "Return's asset balance for owner.",
            :examples [{:code "(balance asset-address owner)"}]
            :type :function
            :signature [{:params [asset-address owner]}]}}
    [asset-address owner]
    (call (address asset-address) (balance owner)))

Perhaps there should be an explicit cast (balance (address owner)) in the final line? This would allow users to be a bit looser in address formats (can use literal blobs) and mean that the SPI can assume an actual address?

Weird title, but this is not legal scientific notation, the exponent must be an integer:

1.0e5.1234

The Reader gets confused and returns the decimal part of that kind of exponent instead of detecting an invalid number:

0.1234

The Compiler handles the case where no argument is provided to unquote-splicing by returning the Context in an exceptional state: https://github.com/Convex-Dev/convex/blob/master/src/main/java/convex/core/lang/Compiler.java#L275

However, upstream execution does not envision that the Context could indeed be in an exceptional state which throws a fatal error:

Edit: Actually, exactly the same situation with unquote.

Scrypt should probably have a statement equivalent to the usual (let [...] ....) form in Lisp

Examples:

let (x=1) x+2;
let (a=1, b=a+1) {a * b;};
(1 + let (b=10) b*b)

Might need to think a bit about whether semicolons are mandatory. Probably not? We should perhaps think of this as a let expression rather than a statement?

Calling pow with a non-numeric argument results in a fatal error and breaks the executing account.

Cause: the Core class do not check if arguments are numbers for graceful failure while the RT class does and throws an IllegalArgumentException.

Need to enable reduce functionality for BlobMaps that operates in-order.

Currently the command (source) of an Invoke Transaction is stored, but not its result.

As reminded by this line (https://github.com/Convex-Dev/convex/blob/master/src/main/java/convex/core/data/AString.java#L17), quotes are not double-escaped when writing EDN.

This means that if a string contains quotes, its output disrupts the whole EDN string which cannot be read back (not accurately at least).

I can PR a simple fix however:

1. How is it managed API-wise when the result of a transaction is such a string, in JSON? .toString seems to suffer from the same problem. Is it relevant? I cannot find the answer in the codebase but it seems to behave well.

2. Isn't it a problem at the level of the Reader? For instance, the code example from actor? in core-metadata.doc:

Source file:

(actor? \"1Ba377262D7637068C8a84b732e30d3Ff62bA891\")

After reading the file, Java string, single-escaped quotes become double-escaped:

"\"(actor? \\\"1Ba377262D7637068C8a84b732e30d3Ff62bA891\\\")\""

After feeding to Reader, single-escaped again:

#object[convex.core.data.StringShort 0x1f11cb95 "(actor? \"1Ba377262D7637068C8a84b732e30d3Ff62bA891\")"]

Should be a Scrypt equivalent to call and deploy in Lisp.

Something like:

call actor-address function_name(arg1,arg2)
call actor-address offer 10000 function_name(arg1,arg2)

Convex should allow 'import' as a statement to create an alias to actor / library code.

Intended usage

(import "adc7f06290972f6aafee38509cf662c42d13dcfed128dc99dc7ceb7414c71062" :as my.library)

(my.library/do-stuff 1 2 3)

Convex should make use of a PKCS #12 key store

Key points:

• Should work with standard Java Key Store tools
• Should be protected by default with a decent key phrase
• Should be stored in the '.convex' directory for the current user on the local machine

Seems broken at present.

recur should allow recurring with different arity of the same function.

The length of a input hexstring for a Blob should be of even length. However, it seems the Reader is buggy and can't handle odd length at all:

0xfff

ERROR (UNDECLARED)
'xfff' is undeclared.

This is certainly confusing for the user.

On a similar note, fix to #54 works but the cast error remains cryptic to the user. Maybe hinting that a hexstring must be of even length? Otherwise it gives the impression that strings in general cannot be cast to blobs.

(blob "f")

ERROR (CAST)
Cast error: Can't convert f of class convex.core.data.StringShort to class class convex.core.data.ABlob.

Namely:

(= NaN NaN)

;; True

The standard defines that this must be always false.

Using scientific notation mEx, all common languages allow m to be an integer.

Eg. 5e2

Convex Reader understands only double m:

5.0e2 ;; => OK

5e2 ;; => Error, "e2" is undeclared

Since CVM doubles rely on actual doubles and floating math, it is possible to produce Infinity:

(/ 1 0)   ; => Infinity
(/ -1 0)   ; => -Infinity

Since they are valid results, it should be accessible to the user just like NaN currently is.

Fix: Adding Infinity and -Infinity in Symbols and register them as special symbols in Core.

Need to implement staking functions in the runtime library.

Key operations:

• Adjust stake (as peer)
• Delegate stake (as any user / actor)
• Claim staking rewards

Both actor? and account? should be used with an argument that is castable to an address, as described in their docstring. However, while account? throws when this is not the case, actor? actually accepts all kind of arguments.

Not a huge deal but inconsistent, so preferably both would behave the same (either solution).

Otherwise it might make it slightly harder for beginners to grasp the difference between actor?, account?, and address?.

Currently it is possible to create an account directly with a transfer. We should consider if this is wise, and perhaps require an explicit create-account operation (which might require a signature for the target account)

Rationale:

• Prevents mistakes by transferring to non-existent accounts
• Consistent with token implementations that rely on an existing account to use set-holding
• Increase transparency on who created accounts

Downsides:

• Extra complexity when making transfers to new accounts. Can probably be handled by clients?

We should be able to support tail calls on the CVM with a simple trampoline at virtually no excess cost (as we do with halt and rollback.

This should make some recursive algorithms much more practical.

Should probably be explicit (core function?)

Destructuring a non-sequential value like 42 fails gracefully, but destructuring a set (specifically) results in a fatal error and breaks the executing account.

Unlike in other languages, those two functions cast NaN and +-Infinity to 0 producing this kind of inconsistent behavior:

(= 1 (inc NaN))  ;;  A

(= NaN (+ 1 NaN))  ;;  B

This is due to the fact that in Java, casting those to long indeed produces 0. Indeed:

(= 0 (long NaN))  ;; Valid by itself, expected on the JVM

However, B is the right way to handle it. Other math functions seem to behave well because they don't cast doubles to longs (they do the opposite if needed), meaning that those special values are left intact and follow proper float arithmetics.

Calling blob with a hexstring of odd length results in a fatal error and breaks the executing account.