Dex, vesting + finish supply/redeem/swap #501
Conversation
| @@ -593,9 +593,9 @@ class SmartContract { | |||
| address: PublicKey; | |||
| tokenId: Field; | |||
|
|
|||
| private _executionState: ExecutionState | undefined; | |||
| #executionState: ExecutionState | undefined; | |||
There was a problem hiding this comment.
changes in this file are to work around some TS glitches when returning a SmartContract from a function (i.e., using a class factory)
| }, | ||
| setGlobalSlotSinceHardfork(slot: UInt32) { | ||
| networkState.globalSlotSinceHardFork = slot; | ||
| incrementGlobalSlot(increment: UInt32 | number) { |
There was a problem hiding this comment.
improvement to the API of setting the global slot: globalSlotSinceGenesis and globalSlotSinceHardfork should always be updated together, not independent of each other. Also, the only update that makes sense is increasing them.
There was a problem hiding this comment.
Does it matter to add UInt64 to this type as well?
There was a problem hiding this comment.
I don't think so, slots are always just UInt32 in Mina
| @@ -940,7 +940,8 @@ class AccountUpdate implements Types.AccountUpdate { | |||
| // if the fee payer is the same account update as this one, we have to start the nonce predicate at one higher, | |||
| // bc the fee payer already increases its nonce | |||
| let isFeePayer = Mina.currentTransaction()?.sender?.equals(publicKey); | |||
| if (isFeePayer?.toBoolean()) nonce++; | |||
| let shouldIncreaseNonce = isFeePayer?.and(tokenId.equals(TokenId.default)); | |||
There was a problem hiding this comment.
small bug fix here when computing nonces in an edge case
|
Note: tests are failing because the bindings are not updated, to make review easier |
mitschabaude
changed the title
| // FIXME | ||
| // Error: Constraint unsatisfied (unreduced): Equal 0 1 |
There was a problem hiding this comment.
Should we remove these comments now that the line below has been enabled? I'm also curious to know what was causing this error if you know the cause?
There was a problem hiding this comment.
I didn't test the line again with proving enabled - just wanted it enabled for fuller testing in the non proving case. All of these tests only work with not proving by now. I have an issue open just for investigating this line: #499
| * Thus, a liquidity provider would need to wait for their current tokens to unlock before being able to | ||
| * supply liquidity again (or, create another account to supply liquidity from). | ||
| */ | ||
| let amountLocked = dl; |
There was a problem hiding this comment.
Should we spin up a ticket to add a nicer API for updating timing functionality?
There was a problem hiding this comment.
It's covered by this one: #186
| }, | ||
| setGlobalSlotSinceHardfork(slot: UInt32) { | ||
| networkState.globalSlotSinceHardFork = slot; | ||
| incrementGlobalSlot(increment: UInt32 | number) { |
There was a problem hiding this comment.
Does it matter to add UInt64 to this type as well?
| /** | ||
| * - Resulting operation will overflow the SC’s receiving token by type or by any other applicable limits; | ||
| * | ||
| * note: this throws not at the protocol level, but because the smart contract multiplies two UInt64s which overflow. |
There was a problem hiding this comment.
what do you mean here? There is a constraint failure due to the overflow, right? It's just due to the smart contract.
There was a problem hiding this comment.
Yes there is a constraint failure. So we don't get to the point where we could trigger an overflow failure in the protocol logic. In that sense, this is not a proper test as it was intended
This PR finishes all remaining tests in the categories "Supply liquidity", "Redeem liquidity", and "Swap".
See comments in
dex/run.tsfor details on how the tests were done, and the testing Google doc for comments about why certain tests were left out.timingfield from accounts on local blockchain