The bug is fixed and the cycles ledger is reinstalled with the latest version. The canister is running again
5 Likes
Maybe one more? ICRC3 specifies Nats for amount and fees and timestamps, but the current block 3 has Ints. @mariop
1 Like
We are aware and we will patch it sometime soon. The issue is caused by the way the serializing library ciborium works with numbers, which results in our Ledgers using int for positive numbers instead of nat. The fix requires some work. In the meanwhile clients of the cycles-ledger will have to interpret ints as nats. Note that out internal library is calculating the hash of positive ints as hash of the leb128 representation which is also different from the ICRC-3 specification but has the benefit that the hash is already correct despite the type being different. The fix will just have to switch int for nat.
This kind of inconsistencies can happen when the standard and the implementation are developed in parallel. The reason why we decided to proceed anyway is that the issue affects only certain clients off-chain that need to verify the Ledger.
2 Likes
Ugh…ok…so Will they stay Ints on this are or they going to get fixed to nats in the get_block response? I have implemented rep indy hash in Typescript and will be calling hashes client side…do I need to do some custom code here?
In motoko I’m slebing all Ints…not just positive ones:
Correct or incorrect? I’m assuming what you are saying is that the the rust code is doing it wrong for positive ints and that helps you fix it because they are supposed use leb anyway?
1 Like
Will they stay Ints on this are or they going to get fixed to nats in the get_block response?
They will get fixed to nats. The cycles-ledger will be fully ICRC-3 compatible.
do I need to do some custom code here?
No, just wait for the fix.
I’m assuming what you are saying is that the the rust code is doing it wrong for positive ints and that helps you fix it because they are supposed use leb anyway?
Our library is doing it wrong which in this case works in out favor as there is no difference between int and nat for block hashing. A correct implementation should always use sleb128 for int.
1 Like
The fix is deployed. Since the total supply was only 700M cycles we decided it’s worth reinstalling (one last time?) instead of maintaining even a few lines of backwards compatibility code for the rest of time
Command dfx cycles --ic seems to be working just fine. So is cycle ledger considered launched? Can people start using it without worrying funds disappearing?
We consider it beta-launched. The dev team still holds control over the canister, so it is still possible for us to change the code without NNS approval, but we plan on handing over control soon 
We have not deployed the index canister yet (still under development), so that part is not tested live yet, but we expect that things should go smoothly. I personally would be happy to store $XXX worth of cycles in there, but I have enough insider knowledge so that I trust the team.
2 Likes
Wait, so I shouldn’t be custodying $$$'s of cycles in there, yet?
Depends on how much you trust me 
Seriously speaking: we (the cycles ledger team, 3 people total) still have control over the canister, so we could always rugpull you. But the code is now stable and we don’t have any more security relevant work planned. Next week we’ll discuss if we can finally hand the canister over to the NNS and remove ourselves as controllers.
2 Likes
Too late. All our cycles are with you now 
P.S.
We found another cool hack that we’re using for a UI right now.
Previously we relied on the Cycles Wallet UI for our non-CLI users in case they needed to topup canisters. But that’s been a bit wonky for some time now. It looks like it’s not widely supported anymore without any updates.
So, instead, what we’re doing for a UI for cycles is use the Candid UI here and open the Cycles Ledger address and then use II to login and obtain a principal and then transfer cycles on the cycles ledger to that principal that can be checked with the icrc1_balance_of method.
And subsequently they can use the withdraw method to top up canisters with this UI. No need to wait for the NNS support to land.
@Severin
Quick question, any reason why the Candid UI Internet Identity uses identity.internetcomputer.org instead of identity.ic0.app?
1 Like
See this for context:
1 Like
Wanted to check in on this to see how close we are to an NNS controlled cycles ledger.
Will the cycle ledger be NNS controlled before the index canister gets deployed, or visa-versa?
We ran into some serious problems with verifiability. If there’s no 100% guaranteed way for people to verify the cycles ledger we won’t hand it over. We’ll have a plan ready early next week.
TLDR: We will reinstall one last time. Please withdraw your cycles.
Long version:
Hello everyone, bad news with verifiability.
The problem:
The first few versions of the cycles ledger we deployed were not reproducible builds, meaning that it is not obvious that there are no hidden balances or approvals in the state. Obviously, we can’t hand the cycles ledger over to the NNS and expect people to trust it this way.
Options considered:
The two options we have moving forward are 1) reinstalling the cycles ledger with verifiable code or 2) add extra endpoints to the cycles ledger and develop tooling that allows everyone to verify that the state is correct. Option 2 would take a good amount of development time but verification would remain complex: To trust the cycles ledger you wouldn’t only have to verify the cycles ledger code, but also the verification protocol and the tools used for that. We concluded that option 1 causes less pain.
The way forward:
Since there is a significant amount of cycles in the cycles ledger (at the time of writing a bit over 51k TC), we execute the following migration plan:
- Today (May 29):
- We have disabled the deposit function so that people don’t deposit more cycles.
- I will reach out to @icme and @saikatdas0790 since they seem to be the two people most affected.
- Today until ~June 3: Please withdraw your cycles!
- ~June 3 (next Monday):
- We disable all endpoints that allow modifying the state of the cycles ledger.
- We make a snapshot of all balances.
- We hand over control of the cycles ledger canister and index canister over to the NNS.
- We submit a proposal to reinstall the cycles ledger.
- We submit a proposal to reinstall the index canister in order to wipe its state so it doesn’t supply outdated information.
- Once the proposal is adopted and executed (probably ~3 days after submitting):
- We refund the balances of everyone that lost their balance using the snapshot from June 3
9 Likes
Thanks for the communication Severin.
We’ve alerted all CycleOps customers that are currently using the Cycle Ledger, and sent out this PSA to get the word out to developers.
2 Likes
Status update:
- Everything is going as planned
- Thank you everyone for withdrawing so diligently! We went down from ~51k TC down to 90 TC in total supply
- We disabled all endpoints that allow modifying the state
- We made the NNS Root canister the sole controller of cycles ledger and cycles index canister
- We made a new state snapshot with all remaining balances
- We submitted proposal 130151 to reinstall the cycles ledger
- We submitted proposal 130152 to reinstall the cycles index
Hi @Severin,
Noticed Dfinity voted to refuse the 2 reinstall proposals. I did the build verification and was able to reproduce both the Index and the Ledger.
But the Arguments hash are not matching, in neither proposal. Maybe that was the reason Dfinity refused?
Can you share what was the reason and how that changes the original plan?
Thanks,
Tiago
Printscreens of my results:
